diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index af86ebc74022..68f579cc7dd4 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -3010,14 +3010,35 @@ HTMLEditor::GetSelectedElement(const nsAtom* aTagName, iter->Init(firstRange); RefPtr lastElementInRange; - for (; !iter->IsDone(); iter->Next()) { + for (nsINode* lastNodeInRange = nullptr; !iter->IsDone(); iter->Next()) { if (lastElementInRange) { // When any node follows an element node, not only one element is // selected so that return nullptr. return nullptr; } - lastElementInRange = Element::FromNodeOrNull(iter->GetCurrentNode()); + // This loop ignored any non-element nodes before first element node. + // Its purpose must be that this method allow to this case as selecting + // an element: + // -

abc d[ef}

+ // because children of an element node is listed up before the element. + // However, this case must not be expected by the initial developer: + // -

a[bc def}

+ // When we meet non-parent and non-next-sibling node of previous node, + // it means that the range across element boundary (open tag in HTML + // source). So, in this case, we should not say only the following + // element is selected. + nsINode* currentNode = iter->GetCurrentNode(); + MOZ_ASSERT(currentNode); + if (lastNodeInRange && + lastNodeInRange->GetParentNode() != currentNode && + lastNodeInRange->GetNextSibling() != currentNode) { + return nullptr; + } + + lastNodeInRange = currentNode; + + lastElementInRange = Element::FromNodeOrNull(lastNodeInRange); if (!lastElementInRange) { continue; } diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index 04b604d912bf..4b3d74d29632 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -1068,33 +1068,32 @@ protected: // Shouldn't be used by friend classes nsINode& aNode) const; /** - * GetSelectedElement() returns an element node which is in first range of - * Selection. The rule is a little bit complicated and the rules do not - * make sense except in a few cases. If you want to use this newly, - * you should create new method instead. This needs to be here for - * comm-central. - * The rules are: - * 1. If Selection selects an element node, i.e., both containers are - * same node and start offset and end offset is start offset + 1. - * (XXX However, if last child is selected, this path is not used.) - * 2. If the argument is "href", look for anchor elements whose href - * attribute is not empty from container of anchor/focus of Selection - * to element. Then, both result are same one, returns the node. - * (i.e., this allows collapsed selection.) - * 3. If the Selection is collapsed, returns null. - * 4. Otherwise, listing up all nodes with content iterator (post-order) - * and only when the last node is first element node, returns the - * element node. + * GetSelectedElement() returns a "selected" element node. "selected" means: + * - there is only one selection range + * - the range starts from an element node or in an element + * - the range ends at immediately after same element + * - and the range does not include any other element nodes. + * Additionally, only when aTagName is nsGkAtoms::href, this thinks that an + * element which has non-empty "href" attribute includes the range, the + * element is selected. * - * @param aTagName The atom of tag name in lower case. - * If nullptr, look for any element node. - * If nsGkAtoms::href, look for an element - * which has non-empty href attribute. - * If nsGkAtoms::anchor or atomized "namedanchor", - * look for an element which has non-empty - * name attribute. - * @param aRv Returns error code. - * @return An element in first range of Selection. + * NOTE: This method is implementation of nsIHTMLEditor.getSelectedElement() + * and comm-central depends on this behavior. Therefore, if you need to use + * this method internally but you need to change, perhaps, you should create + * another method for avoiding breakage of comm-central apps. + * + * @param aTagName The atom of tag name in lower case. Set this to + * result of GetLowerCaseNameAtom() if you have a tag + * name with nsString. + * If nullptr, this returns any element node or nullptr. + * If nsGkAtoms::href, this returns an element which + * has non-empty "href" attribute or nullptr. + * If nsGkAtoms::anchor, this returns an element which + * has non-empty "name" attribute or nullptr. + * Otherwise, returns an element node whose name is + * same as aTagName or nullptr. + * @param aRv Returns error code. + * @return A "selected" element. */ already_AddRefed GetSelectedElement(const nsAtom* aTagName, diff --git a/editor/libeditor/tests/test_nsIHTMLEditor_getSelectedElement.html b/editor/libeditor/tests/test_nsIHTMLEditor_getSelectedElement.html index 19f483c3a132..346a7d2ec9b9 100644 --- a/editor/libeditor/tests/test_nsIHTMLEditor_getSelectedElement.html +++ b/editor/libeditor/tests/test_nsIHTMLEditor_getSelectedElement.html @@ -210,12 +210,12 @@ SimpleTest.waitForFocus(async function() { selection.removeAllRanges(); selection.addRange(range); - todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")), - null, - "#1-7 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection ends the element but starts from the previous text node"); - todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")), - null, - "#1-7 nsIHTMLEditor::getSelectedElement(\"b\") should return null when Selection ends the element but starts from the previous text node"); + is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")), + null, + "#1-7 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection ends the element but starts from the previous text node"); + is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")), + null, + "#1-7 nsIHTMLEditor::getSelectedElement(\"b\") should return null when Selection ends the element but starts from the previous text node"); is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("i")), null, "#1-7 nsIHTMLEditor::getSelectedElement(\"i\") should return null when Selection ends the element but starts from the previous text node"); @@ -670,9 +670,9 @@ SimpleTest.waitForFocus(async function() { selection.removeAllRanges(); selection.addRange(range); - todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")), - null, - "#6-1 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection starts from previous text node of element and end at end of element"); + is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")), + null, + "#6-1 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection starts from previous text node of element and end at end of element"); is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("href")), null, "#6-1 nsIHTMLEditor::getSelectedElement(\"href\") should return null when Selection starts from previous text node of element and end at end of element"); diff --git a/editor/nsIHTMLEditor.idl b/editor/nsIHTMLEditor.idl index 2db7b7c8dafb..3dab967d097e 100644 --- a/editor/nsIHTMLEditor.idl +++ b/editor/nsIHTMLEditor.idl @@ -319,25 +319,24 @@ interface nsIHTMLEditor : nsISupports in Node aNode); /** - * Return an Element only if it is the only node selected, - * such as an image, horizontal rule, etc. The return type is - * nsISupports for implementation convenience; the returned object, - * if not null, is always a DOM Element. + * getSelectedElement() returns a "selected" element node. "selected" means: + * - there is only one selection range + * - the range starts from an element node or in an element + * - the range ends at immediately after same element + * - and the range does not include any other element nodes. + * Additionally, only when aTagName is "href", this thinks that an + * element which has non-empty "href" attribute includes the range, the + * element is selected. * - * The exception is a link, which is more like a text attribute: - * The Anchor tag is returned if the selection is within the textnode(s) - * that are children of the "A" node. - * This could be a collapsed selection, i.e., a caret - * within the link text. - * - * @param aTagName The HTML tagname or and empty string - * to get any element (but only if it is the only element selected) - * Special input values for Links and Named anchors: - * Use "href" to get a link node - * (an "A" tag with the "href" attribute set) - * Use "anchor" or "namedanchor" to get a named anchor node - * (an "A" tag with the "name" attribute set) - * @return the element as described above + * @param aTagName Case-insensitive element name. + * If empty string, this returns any element node or null. + * If "href", this returns an element which has + * non-empty "href" attribute or null. + * If "anchor", this returns an element which has + * non-empty "name" attribute or null. + * Otherwise, returns an element node whose name is + * same as aTagName or null. + * @return A "selected" element. */ nsISupports getSelectedElement(in AString aTagName);