Bug 1501663 - part 2: HTMLEditor::GetSelectedElement() should return nullptr if 2 or more elements are in selected range r=m_kato

This is also regression of bug 1432944, but hidden by the optimization of
bug 1482019.

In bug 1482019, we removing the code clearing |found| flag when the loop
meets second element in selection range.
https://searchfox.org/mozilla-central/diff/0dd29e18302c5e6b07b88aac7164889d752acda7/editor/libeditor/HTMLEditor.cpp#2751-2753
because the flag won't be referred.

However, before the fix of bug 1432944, it's referred and the cleared flag
makes the method return nullptr.

Therefore, this patch makes it return nullptr when 2 or more elements are
in selected range.

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Masayuki Nakano 2018-11-05 04:57:02 +00:00
parent fa95dc80c3
commit 2654031504
3 changed files with 36 additions and 48 deletions

View File

@ -3014,12 +3014,9 @@ HTMLEditor::GetSelectedElement(const nsAtom* aTagName,
nsCOMPtr<nsIContentIterator> iter = NS_NewContentIterator();
bool found = !!selectedElement;
const nsAtom* tagNameLookingFor = aTagName;
iter->Init(firstRange);
// loop through the content iterator for each content node
while (!iter->IsDone()) {
// Update selectedElement with new node. If it's not an element node,
// clear it.
// XXX This is really odd since this means that the result depends on
// what is the last node. If the last node is an element node,
// it may be returned even if it does not match with aTagName.
@ -3028,33 +3025,24 @@ HTMLEditor::GetSelectedElement(const nsAtom* aTagName,
// name explains.
selectedElement = Element::FromNodeOrNull(iter->GetCurrentNode());
if (selectedElement) {
// If we already found a node, then we have another element,
// thus there's not just one element selected.
// XXX Really odd. The new element node may be different name element.
// So, this means that we return any next element node if we find
// proper element as first element in the range.
if (found) {
break;
// At least 2 elements are in the range so that return nullptr.
return nullptr;
}
if (!tagNameLookingFor) {
// Get name of first selected element
// XXX Looks like that this is necessary only for making the following
// handler work as expected... Why don't you check this below??
tagNameLookingFor = selectedElement->NodeInfo()->NameAtom();
if (!aTagName) {
found = true;
}
// The "A" tag is a pain,
// used for both link(href is set) and "Named Anchor"
if ((isLinkTag &&
else if ((isLinkTag &&
HTMLEditUtils::IsLink(selectedElement)) ||
(isNamedAnchorTag &&
HTMLEditUtils::IsNamedAnchor(selectedElement))) {
found = true;
}
// All other tag names are handled here.
else if (tagNameLookingFor ==
selectedElement->NodeInfo()->NameAtom()) {
else if (aTagName == selectedElement->NodeInfo()->NameAtom()) {
found = true;
}

View File

@ -1082,9 +1082,9 @@ protected: // Shouldn't be used by friend classes
* 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).
* 4-1. When first element node matches with the argument, returns
* *next* element node.
* 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.
*
* @param aTagName The atom of tag name in lower case.
* If nullptr, look for any element node.

View File

@ -236,10 +236,10 @@ SimpleTest.waitForFocus(async function() {
selection.removeAllRanges();
selection.addRange(range);
todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
null,
"#1-8 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection includes 2 elements and starts from previous text node");
todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")),
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")),
null,
"#1-8 nsIHTMLEditor::getSelectedElement(\"b\") should return null when Selection includes 2 elements and starts from previous text node");
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("i")),
@ -345,10 +345,10 @@ SimpleTest.waitForFocus(async function() {
selection.removeAllRanges();
selection.addRange(range);
todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
null,
"#2-1 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection is across 3 <b> elements");
todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")),
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")),
null,
"#2-1 nsIHTMLEditor::getSelectedElement(\"b\") should return null when Selection is across 3 <b> elements");
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("i")),
@ -362,10 +362,10 @@ SimpleTest.waitForFocus(async function() {
selection.removeAllRanges();
selection.addRange(range);
todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
null,
"#2-2 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection is across 3 <b> elements and previous text node");
todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")),
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")),
null,
"#2-2 nsIHTMLEditor::getSelectedElement(\"b\") should return null when Selection is across 3 <b> elements and previous text node");
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("i")),
@ -416,10 +416,10 @@ SimpleTest.waitForFocus(async function() {
selection.removeAllRanges();
selection.addRange(range);
todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("")),
null,
"#3-3 nsIHTMLEditor::getSelectedElement(\"\") should return null when Selection is across 3 <b> elements which are nested and following text node");
todo_is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")),
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("b")),
null,
"#3-3 nsIHTMLEditor::getSelectedElement(\"b\") should return null when Selection is across 3 <b> elements which are nested and following text node");
is(SpecialPowers.unwrap(getHTMLEditor().getSelectedElement("i")),