Bug 1501663 - part 7: HTMLEditor::GetSelectedElement() shouldn't return element node when Selection starts before the element node r=m_kato

This fixes odd case of this API.  GetSelectedElement() ignores non-element
nodes before an element node.  This must be intended to allow Selection to
start from in an element node.  However, current code allows to select starting
from previous text node.  This patch changes this behavior to stop looking
for element node if non-element node appears before an element node.

Differential Revision: https://phabricator.services.mozilla.com/D10703

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Masayuki Nakano 2018-11-05 11:38:10 +00:00
Родитель 189f0a35de
Коммит 2bde324acf
4 изменённых файлов: 74 добавлений и 55 удалений

Просмотреть файл

@ -3010,14 +3010,35 @@ HTMLEditor::GetSelectedElement(const nsAtom* aTagName,
iter->Init(firstRange);
RefPtr<Element> 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:
// - <p>abc <b>d[ef</b>}</p>
// because children of an element node is listed up before the element.
// However, this case must not be expected by the initial developer:
// - <p>a[bc <b>def</b>}</p>
// 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;
}

Просмотреть файл

@ -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 <body> 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
* <a> element which has non-empty "href" attribute includes the range, the
* <a> 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 <a> element
* which has non-empty href attribute.
* If nsGkAtoms::anchor or atomized "namedanchor",
* look for an <a> 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 <a> element which
* has non-empty "href" attribute or nullptr.
* If nsGkAtoms::anchor, this returns an <a> 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<Element>
GetSelectedElement(const nsAtom* aTagName,

Просмотреть файл

@ -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 <b> 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 <b> 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 <b> 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 <b> 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 <b> 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 <b> element and end at end of <b> element");
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
null,
"#6-1 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection starts from previous text node of <b> element and end at end of <b> element");
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("href")),
null,
"#6-1 nsIHTMLEditor::getSelectedElement(\"href\") should return null when Selection starts from previous text node of <b> element and end at end of <b> element");

Просмотреть файл

@ -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 <a>
* element which has non-empty "href" attribute includes the range, the
* <a> 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 <a> element which has
* non-empty "href" attribute or null.
* If "anchor", this returns an <a> 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);