From c0e01ee34dcd60e080b0aed8388504d0c64223c6 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 16 Nov 2010 15:45:49 -0500 Subject: [PATCH] Bug 612128 - Prevent the editor from modifying nodes which are not under an editing host; r=roc,bzbarsky This patch ensures that the NODE_IS_EDITABLE flag is only set on nodes living under an editing host. Things like text controls which used to have that flag previously will not have it any more. The flag would be set on their anonymous div node instead. Note that if text controls actually fall under an editing host, they will get the NODE_IS_EDITABLE flag. This patch also makes nsHTMLEditor::IsEditable return sane results (text nodes are always considered to be editable). --- .../html/content/src/nsGenericHTMLElement.cpp | 43 ++++---------- .../html/content/src/nsGenericHTMLElement.h | 2 - .../html/content/src/nsHTMLInputElement.cpp | 1 - content/html/content/src/nsHTMLInputElement.h | 5 -- .../content/src/nsHTMLTextAreaElement.cpp | 8 --- .../html/content/src/nsTextEditorState.cpp | 3 + .../libeditor/base/crashtests/382527-1.html | 4 +- editor/libeditor/base/nsEditor.h | 2 +- editor/libeditor/base/nsEditorCommands.cpp | 9 +-- editor/libeditor/html/nsHTMLDataTransfer.cpp | 4 +- editor/libeditor/html/nsHTMLEditor.cpp | 25 ++++++++ editor/libeditor/html/nsHTMLEditor.h | 1 + editor/libeditor/html/tests/Makefile.in | 1 + .../libeditor/html/tests/test_bug612128.html | 42 ++++++++++++++ .../libeditor/html/tests/test_bug676401.html | 9 ++- layout/base/nsCSSFrameConstructor.cpp | 58 ++++++++++++------- layout/generic/nsIAnonymousContentCreator.h | 4 ++ .../editor/readonly-editable-ref.html | 13 +++++ layout/reftests/editor/readonly-editable.html | 24 ++++++++ .../editor/readonly-non-editable-ref.html | 21 +++++++ .../editor/readonly-non-editable.html | 24 ++++++++ .../editor/readwrite-editable-ref.html | 13 +++++ .../reftests/editor/readwrite-editable.html | 24 ++++++++ .../editor/readwrite-non-editable-ref.html | 21 +++++++ .../editor/readwrite-non-editable.html | 24 ++++++++ layout/reftests/editor/reftest.list | 4 ++ 26 files changed, 310 insertions(+), 79 deletions(-) create mode 100644 editor/libeditor/html/tests/test_bug612128.html create mode 100644 layout/reftests/editor/readonly-editable-ref.html create mode 100644 layout/reftests/editor/readonly-editable.html create mode 100644 layout/reftests/editor/readonly-non-editable-ref.html create mode 100644 layout/reftests/editor/readonly-non-editable.html create mode 100644 layout/reftests/editor/readwrite-editable-ref.html create mode 100644 layout/reftests/editor/readwrite-editable.html create mode 100644 layout/reftests/editor/readwrite-non-editable-ref.html create mode 100644 layout/reftests/editor/readwrite-non-editable.html diff --git a/content/html/content/src/nsGenericHTMLElement.cpp b/content/html/content/src/nsGenericHTMLElement.cpp index e88523336a98..72c40c51f3c6 100644 --- a/content/html/content/src/nsGenericHTMLElement.cpp +++ b/content/html/content/src/nsGenericHTMLElement.cpp @@ -1789,37 +1789,6 @@ nsGenericHTMLElement::MapCommonAttributesInto(const nsMappedAttributes* aAttribu } } -void -nsGenericHTMLFormElement::UpdateEditableFormControlState(PRBool aNotify) -{ - // nsCSSFrameConstructor::MaybeConstructLazily is based on the logic of this - // function, so should be kept in sync with that. - - ContentEditableTristate value = GetContentEditableValue(); - if (value != eInherit) { - DoSetEditableFlag(!!value, aNotify); - return; - } - - nsIContent *parent = GetParent(); - - if (parent && parent->HasFlag(NODE_IS_EDITABLE)) { - DoSetEditableFlag(PR_TRUE, aNotify); - return; - } - - if (!IsTextControl(PR_FALSE)) { - DoSetEditableFlag(PR_FALSE, aNotify); - return; - } - - // If not contentEditable we still need to check the readonly attribute. - PRBool roState; - GetBoolAttr(nsGkAtoms::readonly, &roState); - - DoSetEditableFlag(!roState, aNotify); -} - /* static */ const nsGenericHTMLElement::MappedAttributeEntry nsGenericHTMLElement::sCommonAttributeMap[] = { @@ -2912,6 +2881,18 @@ nsGenericHTMLFormElement::IntrinsicState() const state |= NS_EVENT_STATE_DEFAULT; } + // Make the text controls read-write + if (!state.HasState(NS_EVENT_STATE_MOZ_READWRITE) && + IsTextControl(PR_FALSE)) { + PRBool roState; + GetBoolAttr(nsGkAtoms::readonly, &roState); + + if (!roState) { + state |= NS_EVENT_STATE_MOZ_READWRITE; + state &= ~NS_EVENT_STATE_MOZ_READONLY; + } + } + return state; } diff --git a/content/html/content/src/nsGenericHTMLElement.h b/content/html/content/src/nsGenericHTMLElement.h index f152e1df360f..b4d1ccfb21cf 100644 --- a/content/html/content/src/nsGenericHTMLElement.h +++ b/content/html/content/src/nsGenericHTMLElement.h @@ -928,8 +928,6 @@ protected: virtual nsresult AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, const nsAString* aValue, PRBool aNotify); - void UpdateEditableFormControlState(PRBool aNotify); - /** * This method will update the form owner, using @form or looking to a parent. * diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp index 2895a9855f8c..c292fd9e5cc2 100644 --- a/content/html/content/src/nsHTMLInputElement.cpp +++ b/content/html/content/src/nsHTMLInputElement.cpp @@ -848,7 +848,6 @@ nsHTMLInputElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, UpdateTypeMismatchValidityState(); } - UpdateEditableState(aNotify); UpdateState(aNotify); } diff --git a/content/html/content/src/nsHTMLInputElement.h b/content/html/content/src/nsHTMLInputElement.h index cd40e7db9069..5c5108adb79d 100644 --- a/content/html/content/src/nsHTMLInputElement.h +++ b/content/html/content/src/nsHTMLInputElement.h @@ -235,11 +235,6 @@ public: NS_IMETHOD FireAsyncClickHandler(); - virtual void UpdateEditableState(PRBool aNotify) - { - return UpdateEditableFormControlState(aNotify); - } - NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsHTMLInputElement, nsGenericHTMLFormElement) diff --git a/content/html/content/src/nsHTMLTextAreaElement.cpp b/content/html/content/src/nsHTMLTextAreaElement.cpp index 40ba9795e542..4d3c5d03953e 100644 --- a/content/html/content/src/nsHTMLTextAreaElement.cpp +++ b/content/html/content/src/nsHTMLTextAreaElement.cpp @@ -200,11 +200,6 @@ public: NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED - virtual void UpdateEditableState(PRBool aNotify) - { - return UpdateEditableFormControlState(aNotify); - } - NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsHTMLTextAreaElement, nsGenericHTMLFormElement) @@ -1267,9 +1262,6 @@ nsHTMLTextAreaElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, UpdateTooLongValidityState(); } - if (aName == nsGkAtoms::readonly) { - UpdateEditableState(aNotify); - } UpdateState(aNotify); } diff --git a/content/html/content/src/nsTextEditorState.cpp b/content/html/content/src/nsTextEditorState.cpp index 28b24cf6ef24..2c686b0f5774 100644 --- a/content/html/content/src/nsTextEditorState.cpp +++ b/content/html/content/src/nsTextEditorState.cpp @@ -1559,6 +1559,9 @@ nsTextEditorState::CreateRootNode() nsresult nsTextEditorState::InitializeRootNode() { + // Make our root node editable + mRootNode->SetFlags(NODE_IS_EDITABLE); + // Set the necessary classes on the text control. We use class values // instead of a 'style' attribute so that the style comes from a user-agent // style sheet and is still applied even if author styles are disabled. diff --git a/editor/libeditor/base/crashtests/382527-1.html b/editor/libeditor/base/crashtests/382527-1.html index ff33a141794f..2441dcd87b52 100644 --- a/editor/libeditor/base/crashtests/382527-1.html +++ b/editor/libeditor/base/crashtests/382527-1.html @@ -36,7 +36,9 @@ function init3() rng.setEnd(textNode, 1); targetWindow.getSelection().addRange(rng); - targetDocument.execCommand("inserthtml", false, "

"); + try { + targetDocument.execCommand("inserthtml", false, "

"); + } catch(e) {} document.documentElement.removeAttribute("class"); } diff --git a/editor/libeditor/base/nsEditor.h b/editor/libeditor/base/nsEditor.h index 078607ec3041..86909949afef 100644 --- a/editor/libeditor/base/nsEditor.h +++ b/editor/libeditor/base/nsEditor.h @@ -548,7 +548,7 @@ public: virtual PRBool IsContainer(nsIDOMNode *aNode); /** returns PR_TRUE if aNode is an editable node */ - PRBool IsEditable(nsIDOMNode *aNode); + virtual PRBool IsEditable(nsIDOMNode *aNode); virtual PRBool IsTextInDirtyFrameVisible(nsIDOMNode *aNode); diff --git a/editor/libeditor/base/nsEditorCommands.cpp b/editor/libeditor/base/nsEditorCommands.cpp index 590e4cc48bd6..2e1c4757a27b 100644 --- a/editor/libeditor/base/nsEditorCommands.cpp +++ b/editor/libeditor/base/nsEditorCommands.cpp @@ -661,13 +661,8 @@ nsSelectAllCommand::IsCommandEnabled(const char * aCommandName, { NS_ENSURE_ARG_POINTER(outCmdEnabled); - // you can select all if there is an editor (and potentially no contents) - // some day we may want to change this - nsCOMPtr editor = do_QueryInterface(aCommandRefCon); - if (editor) - return editor->GetIsSelectionEditable(outCmdEnabled); - - *outCmdEnabled = PR_FALSE; + // You can always select all! + *outCmdEnabled = PR_TRUE; return NS_OK; } diff --git a/editor/libeditor/html/nsHTMLDataTransfer.cpp b/editor/libeditor/html/nsHTMLDataTransfer.cpp index be2a9e53a357..d9c1671a0669 100644 --- a/editor/libeditor/html/nsHTMLDataTransfer.cpp +++ b/editor/libeditor/html/nsHTMLDataTransfer.cpp @@ -311,7 +311,9 @@ nsHTMLEditor::DoInsertHTMLWithContext(const nsAString & aInputString, // if caller didn't provide the destination/target node, // fetch the paste insertion point from our selection res = GetStartNodeAndOffset(selection, getter_AddRefs(targetNode), &targetOffset); - if (!targetNode) res = NS_ERROR_FAILURE; + if (!targetNode || !IsEditable(targetNode)) { + res = NS_ERROR_FAILURE; + } NS_ENSURE_SUCCESS(res, res); } else diff --git a/editor/libeditor/html/nsHTMLEditor.cpp b/editor/libeditor/html/nsHTMLEditor.cpp index 1275fa3e6a32..eafd6bfe7738 100644 --- a/editor/libeditor/html/nsHTMLEditor.cpp +++ b/editor/libeditor/html/nsHTMLEditor.cpp @@ -431,6 +431,12 @@ nsHTMLEditor::FindSelectionRoot(nsINode *aNode) } if (!content->HasFlag(NODE_IS_EDITABLE)) { + // If the content is in read-write state but is not editable itself, + // return it as the selection root. + if (content->IsElement() && + content->AsElement()->State().HasState(NS_EVENT_STATE_MOZ_READWRITE)) { + return content.forget(); + } return nsnull; } @@ -6028,3 +6034,22 @@ nsHTMLEditor::GetPreferredIMEState(PRUint32 *aState) *aState = nsIContent::IME_STATUS_ENABLE; return NS_OK; } + +PRBool +nsHTMLEditor::IsEditable(nsIDOMNode* aNode) { + if (!nsPlaintextEditor::IsEditable(aNode)) { + return PR_FALSE; + } + nsCOMPtr node = do_QueryInterface(aNode); + if (!node) { + // If what we're dealing with is not a node, then it's not editable! + return PR_FALSE; + } + if (node->IsElement()) { + // If we're dealing with an element, then ask it whether it's editable. + return node->IsEditable(); + } + // We might be dealing with a text node for example, which we always consider + // to be editable. + return PR_TRUE; +} diff --git a/editor/libeditor/html/nsHTMLEditor.h b/editor/libeditor/html/nsHTMLEditor.h index d209bc1b5dcb..a723e5b4988f 100644 --- a/editor/libeditor/html/nsHTMLEditor.h +++ b/editor/libeditor/html/nsHTMLEditor.h @@ -154,6 +154,7 @@ public: virtual already_AddRefed GetDOMEventTarget(); virtual already_AddRefed FindSelectionRoot(nsINode *aNode); virtual PRBool IsAcceptableInputEvent(nsIDOMEvent* aEvent); + virtual PRBool IsEditable(nsIDOMNode *aNode); /* ------------ nsStubMutationObserver overrides --------- */ NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED diff --git a/editor/libeditor/html/tests/Makefile.in b/editor/libeditor/html/tests/Makefile.in index 9558aff18bc3..99f46b3d6c41 100644 --- a/editor/libeditor/html/tests/Makefile.in +++ b/editor/libeditor/html/tests/Makefile.in @@ -80,6 +80,7 @@ _TEST_FILES = \ test_bug599322.html \ test_bug607584.html \ test_bug611182.html \ + test_bug612128.html \ test_bug612447.html \ test_bug620906.html \ test_bug622371.html \ diff --git a/editor/libeditor/html/tests/test_bug612128.html b/editor/libeditor/html/tests/test_bug612128.html new file mode 100644 index 000000000000..660bb54904e4 --- /dev/null +++ b/editor/libeditor/html/tests/test_bug612128.html @@ -0,0 +1,42 @@ + + + + + Test for Bug 612128 + + + + + + +Mozilla Bug 612128 +

+
+ +
+
+
+
+
+ + diff --git a/editor/libeditor/html/tests/test_bug676401.html b/editor/libeditor/html/tests/test_bug676401.html index ba60753f7579..e1915c7b9b8a 100644 --- a/editor/libeditor/html/tests/test_bug676401.html +++ b/editor/libeditor/html/tests/test_bug676401.html @@ -35,10 +35,17 @@ var gBlock1, gBlock2; function IsCommandEnabled(command) { var enabled; + var resultInNonEditableRegion = false; + if (command == "selectAll") { + // The select all command is sort of exceptional, as it needs to be enabled + // everywhere. + resultInNonEditableRegion = true; + } + // non-editable div: should return false window.getSelection().selectAllChildren(gBlock1); enabled = document.queryCommandEnabled(command); - is(enabled, false, "'" + command + "' should not be enabled on a non-editable block."); + is(enabled, resultInNonEditableRegion, "'" + command + "' should not be enabled on a non-editable block."); // editable div: should return true window.getSelection().selectAllChildren(gBlock2); diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index fc2d939f0598..ed61f782ca01 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -3883,6 +3883,31 @@ nsCSSFrameConstructor::CreateAnonymousFrames(nsFrameConstructorState& aState, return NS_OK; } +static void +SetFlagsOnSubtree(nsIContent *aNode, PtrBits aFlagsToSet) +{ +#ifdef DEBUG + // Make sure that the node passed to us doesn't have any XBL children + { + nsIDocument *doc = aNode->GetOwnerDoc(); + NS_ASSERTION(doc, "The node must be in a document"); + NS_ASSERTION(!doc->BindingManager()->GetXBLChildNodesFor(aNode), + "The node should not have any XBL children"); + } +#endif + + // Set the flag on the node itself + aNode->SetFlags(aFlagsToSet); + + // Set the flag on all of its children recursively + PRUint32 count; + nsIContent * const *children = aNode->GetChildArray(&count); + + for (PRUint32 index = 0; index < count; ++index) { + SetFlagsOnSubtree(children[index], aFlagsToSet); + } +} + nsresult nsCSSFrameConstructor::GetAnonymousContent(nsIContent* aParent, nsIFrame* aParentFrame, @@ -3910,7 +3935,17 @@ nsCSSFrameConstructor::GetAnonymousContent(nsIContent* aParent, content->SetNativeAnonymous(); } + PRBool anonContentIsEditable = content->HasFlag(NODE_IS_EDITABLE); rv = content->BindToTree(mDocument, aParent, aParent, PR_TRUE); + // If the anonymous content creator requested that the content should be + // editable, honor its request. + // We need to set the flag on the whole subtree, because existing + // children's flags have already been set as part of the BindToTree operation. + if (anonContentIsEditable) { + NS_ASSERTION(aParentFrame->GetType() == nsGkAtoms::textInputFrame, + "We only expect this for anonymous content under a text control frame"); + SetFlagsOnSubtree(content, NODE_IS_EDITABLE); + } if (NS_FAILED(rv)) { content->UnbindFromTree(); return rv; @@ -6137,25 +6172,6 @@ nsCSSFrameConstructor::ReframeTextIfNeeded(nsIContent* aParentContent, ContentInserted(aParentContent, aContent, nsnull, PR_FALSE); } -// We want to disable lazy frame construction for nodes that are under an -// editor. We use nsINode::IsEditable, but that includes inputs with type text -// and password and textareas, which are common and aren't really editable (the -// native anonymous content under them is what is actually editable) so we want -// to construct frames for those lazily. -// The logic for this check is based on -// nsGenericHTMLFormElement::UpdateEditableFormControlState and so must be kept -// in sync with that. MayHaveContentEditableAttr() being true only indicates -// a contenteditable attribute, it doesn't indicate whether it is true or false, -// so we force eager construction in some cases when the node is not editable, -// but that should be rare. -static inline PRBool -IsActuallyEditable(nsIContent* aContainer, nsIContent* aChild) -{ - return (aChild->IsEditable() && - (aContainer->IsEditable() || - aChild->MayHaveContentEditableAttr())); -} - // For inserts aChild should be valid, for appends it should be null. // Returns true if this operation can be lazy, false if not. PRBool @@ -6170,7 +6186,7 @@ nsCSSFrameConstructor::MaybeConstructLazily(Operation aOperation, if (aOperation == CONTENTINSERT) { if (aChild->IsRootOfAnonymousSubtree() || - aChild->IsXUL() || IsActuallyEditable(aContainer, aChild)) { + aChild->IsEditable() || aChild->IsXUL()) { return PR_FALSE; } } else { // CONTENTAPPEND @@ -6179,7 +6195,7 @@ nsCSSFrameConstructor::MaybeConstructLazily(Operation aOperation, for (nsIContent* child = aChild; child; child = child->GetNextSibling()) { NS_ASSERTION(!child->IsRootOfAnonymousSubtree(), "Should be coming through the CONTENTAPPEND case"); - if (child->IsXUL() || IsActuallyEditable(aContainer, child)) { + if (child->IsXUL() || child->IsEditable()) { return PR_FALSE; } } diff --git a/layout/generic/nsIAnonymousContentCreator.h b/layout/generic/nsIAnonymousContentCreator.h index b1e84dc53057..4c351a99b698 100644 --- a/layout/generic/nsIAnonymousContentCreator.h +++ b/layout/generic/nsIAnonymousContentCreator.h @@ -79,6 +79,10 @@ public: * Creates "native" anonymous content and adds the created content to * the aElements array. None of the returned elements can be nsnull. * + * If the anonymous content creator sets the editable flag on some + * of the elements that it creates, the flag will be applied to the node + * upon being bound to the document. + * * @note The returned elements are owned by this object. This object is * responsible for calling UnbindFromTree on the elements it returned * from CreateAnonymousContent when appropriate (i.e. before releasing diff --git a/layout/reftests/editor/readonly-editable-ref.html b/layout/reftests/editor/readonly-editable-ref.html new file mode 100644 index 000000000000..99f1e5101739 --- /dev/null +++ b/layout/reftests/editor/readonly-editable-ref.html @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/layout/reftests/editor/readonly-editable.html b/layout/reftests/editor/readonly-editable.html new file mode 100644 index 000000000000..49210e581472 --- /dev/null +++ b/layout/reftests/editor/readonly-editable.html @@ -0,0 +1,24 @@ + + + + + + + hide me + hide me + hide me + hide me + hide me + hide me + hide me + hide me + + diff --git a/layout/reftests/editor/readonly-non-editable-ref.html b/layout/reftests/editor/readonly-non-editable-ref.html new file mode 100644 index 000000000000..a91071e42c33 --- /dev/null +++ b/layout/reftests/editor/readonly-non-editable-ref.html @@ -0,0 +1,21 @@ + + + + + + + hide me + + hide me + + hide me + + hide me + + + diff --git a/layout/reftests/editor/readonly-non-editable.html b/layout/reftests/editor/readonly-non-editable.html new file mode 100644 index 000000000000..9766045ed73d --- /dev/null +++ b/layout/reftests/editor/readonly-non-editable.html @@ -0,0 +1,24 @@ + + + + + + + hide me + hide me + hide me + hide me + hide me + hide me + hide me + hide me + + diff --git a/layout/reftests/editor/readwrite-editable-ref.html b/layout/reftests/editor/readwrite-editable-ref.html new file mode 100644 index 000000000000..99f1e5101739 --- /dev/null +++ b/layout/reftests/editor/readwrite-editable-ref.html @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/layout/reftests/editor/readwrite-editable.html b/layout/reftests/editor/readwrite-editable.html new file mode 100644 index 000000000000..49210e581472 --- /dev/null +++ b/layout/reftests/editor/readwrite-editable.html @@ -0,0 +1,24 @@ + + + + + + + hide me + hide me + hide me + hide me + hide me + hide me + hide me + hide me + + diff --git a/layout/reftests/editor/readwrite-non-editable-ref.html b/layout/reftests/editor/readwrite-non-editable-ref.html new file mode 100644 index 000000000000..12e1c46c0aef --- /dev/null +++ b/layout/reftests/editor/readwrite-non-editable-ref.html @@ -0,0 +1,21 @@ + + + + + + + + hide me + + hide me + + hide me + + hide me + + diff --git a/layout/reftests/editor/readwrite-non-editable.html b/layout/reftests/editor/readwrite-non-editable.html new file mode 100644 index 000000000000..535f21f1aa0e --- /dev/null +++ b/layout/reftests/editor/readwrite-non-editable.html @@ -0,0 +1,24 @@ + + + + + + + hide me + hide me + hide me + hide me + hide me + hide me + hide me + hide me + + diff --git a/layout/reftests/editor/reftest.list b/layout/reftests/editor/reftest.list index 4c51f4040478..b9f7c12a75f2 100644 --- a/layout/reftests/editor/reftest.list +++ b/layout/reftests/editor/reftest.list @@ -69,4 +69,8 @@ skip-if(Android) == 674212-spellcheck.html 674212-spellcheck-ref.html skip-if(Android) == 338427-2.html 338427-2-ref.html skip-if(Android) == 338427-3.html 338427-3-ref.html skip-if(Android) == 462758-grabbers-resizers.html 462758-grabbers-resizers-ref.html +== readwrite-non-editable.html readwrite-non-editable-ref.html +== readwrite-editable.html readwrite-editable-ref.html +== readonly-non-editable.html readonly-non-editable-ref.html +== readonly-editable.html readonly-editable-ref.html == dynamic-overflow-change.html dynamic-overflow-change-ref.html