Bug 1496320 - part 1: Make TypeInState clear "link style" when it detects caret coming from outside of current link r=m_kato

Only `TypeInState` stores last caret position.  So, only it can detect the
case caret is moved from outside of `<a href>` element and is now at start
or end of it.

Note that this patch does not assume `<a href>` has an empty text node with
another inline element.  If we supported it, the loop would be more complicated
and it's really unrealistic edge case.  Therefore, it's reasonable to ignore
the case.

And also this patch works with `ArrowUp`/`ArrowDown`/`PageUp`/`PageDown`
cases.  However, I have no idea to reject such cases.  But I guess that
it does not so bad behavior for users because caret does not moved in
a text node in `<a href>`.

Depends on D69479

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Masayuki Nakano 2020-04-06 03:52:53 +00:00
parent 6be7ff90be
commit 5252248a15
5 changed files with 44 additions and 10 deletions

View File

@ -719,7 +719,7 @@ class EditorDOMPointBase final {
if (NS_WARN_IF(!mParent)) {
return false;
}
if (mParent->IsContainerNode()) {
if (mParent->IsContainerNode() && mOffset.isSome()) {
return mOffset.value() == mParent->Length() - 1;
}
if (mIsChildInitialized) {

View File

@ -348,7 +348,7 @@ NS_IMETHODIMP HTMLEditor::NotifySelectionChanged(Document* aDocument,
if (mTypeInState) {
RefPtr<TypeInState> typeInState = mTypeInState;
typeInState->OnSelectionChange(*aSelection);
typeInState->OnSelectionChange(*aSelection, aReason);
// We used a class which derived from nsISelectionListener to call
// HTMLEditor::RefreshEditingUI(). The lifetime of the class was

View File

@ -7,12 +7,14 @@
#include <stddef.h>
#include "nsError.h"
#include "HTMLEditUtils.h"
#include "mozilla/EditorBase.h"
#include "mozilla/mozalloc.h"
#include "mozilla/dom/AncestorIterator.h"
#include "mozilla/dom/Selection.h"
#include "nsAString.h"
#include "nsDebug.h"
#include "nsError.h"
#include "nsGkAtoms.h"
#include "nsINode.h"
#include "nsISupportsBase.h"
@ -76,7 +78,7 @@ nsresult TypeInState::UpdateSelState(Selection* aSelection) {
return NS_OK;
}
void TypeInState::OnSelectionChange(Selection& aSelection) {
void TypeInState::OnSelectionChange(Selection& aSelection, int16_t aReason) {
// XXX: Selection currently generates bogus selection changed notifications
// XXX: (bug 140303). It can notify us when the selection hasn't actually
// XXX: changed, and it notifies us more than once for the same change.
@ -87,6 +89,7 @@ void TypeInState::OnSelectionChange(Selection& aSelection) {
// XXX: This code temporarily fixes the problem where clicking the mouse in
// XXX: the same location clears the type-in-state.
bool unlink = false;
if (aSelection.IsCollapsed() && aSelection.RangeCount()) {
EditorRawDOMPoint selectionStartPoint(
EditorBase::GetStartPoint(aSelection));
@ -99,6 +102,33 @@ void TypeInState::OnSelectionChange(Selection& aSelection) {
return;
}
// If caret comes from outside of <a href> element, we should clear "link"
// style after reset.
if (aReason == nsISelectionListener::KEYPRESS_REASON &&
mLastSelectionPoint.IsSet() && selectionStartPoint.IsInTextNode() &&
(selectionStartPoint.IsStartOfContainer() ||
selectionStartPoint.IsEndOfContainer()) &&
// If we're moving in same text node, we can assume that we should
// stay in the <a href>.
mLastSelectionPoint.GetContainer() !=
selectionStartPoint.GetContainer()) {
// XXX Assuming it's not empty text node because it's unrealistic edge
// case.
bool maybeStartOfAnchor = selectionStartPoint.IsStartOfContainer();
for (EditorRawDOMPoint point(selectionStartPoint.GetContainer());
point.IsSet() && (maybeStartOfAnchor ? point.IsStartOfContainer()
: point.IsAtLastContent());
point.Set(point.GetContainer())) {
// TODO: We should check editing host boundary here.
if (HTMLEditUtils::IsLink(point.GetContainer())) {
// Now, we're at start or end of <a href>.
unlink = !mLastSelectionPoint.GetContainer()->IsInclusiveDescendantOf(
point.GetContainer());
break;
}
}
}
mLastSelectionPoint = selectionStartPoint;
// We need to store only offset because referring child may be removed by
// we'll check the point later.
@ -108,6 +138,10 @@ void TypeInState::OnSelectionChange(Selection& aSelection) {
}
Reset();
if (unlink) {
ClearProp(nsGkAtoms::a, nullptr);
}
}
void TypeInState::Reset() {

View File

@ -88,7 +88,7 @@ class TypeInState final {
nsresult UpdateSelState(dom::Selection* aSelection);
void OnSelectionChange(dom::Selection& aSelection);
void OnSelectionChange(dom::Selection& aSelection, int16_t aReason);
void SetProp(nsAtom* aProp, nsAtom* aAttr, const nsAString& aValue);

View File

@ -62,7 +62,7 @@ SimpleTest.waitForFocus(() => {
selection.collapse(editor.querySelector("p").firstChild, 3);
synthesizeKey("KEY_ArrowRight");
synthesizeKey("X");
todo_is(editor.innerHTML, "<p>abc</p><p>X<a href=\"about:blank\">def</a></p>",
is(editor.innerHTML, "<p>abc</p><p>X<a href=\"about:blank\">def</a></p>",
"Typing X at start of the <a> element should insert to start of new text node when caret is moved from the previous paragraph");
editor.innerHTML = "<p>abc</p><p><b><a href=\"about:blank\">def</a></b></p>";
@ -76,7 +76,7 @@ SimpleTest.waitForFocus(() => {
selection.collapse(editor.querySelector("p").firstChild, 3);
synthesizeKey("KEY_ArrowRight");
synthesizeKey("X");
todo_is(editor.innerHTML, "<p>abc</p><p><b>X<a href=\"about:blank\">def</a></b></p>",
is(editor.innerHTML, "<p>abc</p><p><b>X<a href=\"about:blank\">def</a></b></p>",
"Typing X at start of the <a> element should insert to start of wrapper <b> when caret is moved from the previous paragraph");
editor.innerHTML = "<p>abc</p><p><a href=\"about:blank\">def</a></p>";
@ -161,21 +161,21 @@ SimpleTest.waitForFocus(() => {
selection.collapse(editor.querySelector("p + p").firstChild, 0);
synthesizeKey("KEY_ArrowLeft");
synthesizeKey("X");
todo_is(editor.innerHTML, "<p><a href=\"about:blank\">abc</a>X</p><p>def</p>",
is(editor.innerHTML, "<p><a href=\"about:blank\">abc</a>X</p><p>def</p>",
"Typing X at end of the <a> element should insert to start of new text node when caret is moved from the following paragraph");
editor.innerHTML = "<p><b><a href=\"about:blank\">abc</a></b></p><p>def</p>";
selection.collapse(editor.querySelector("p + p").firstChild, 0);
synthesizeKey("KEY_ArrowLeft");
synthesizeKey("X");
todo_is(editor.innerHTML, "<p><b><a href=\"about:blank\">abc</a>X</b></p><p>def</p>",
is(editor.innerHTML, "<p><b><a href=\"about:blank\">abc</a>X</b></p><p>def</p>",
"Typing X at end of the <a> element should insert to end of wrapper <b> when caret is moved from the following paragraph");
editor.innerHTML = "<p><a href=\"about:blank\"><b>abc</b></a></p><p>def</p>";
selection.collapse(editor.querySelector("p + p").firstChild, 0);
synthesizeKey("KEY_ArrowLeft");
synthesizeKey("X");
todo_is(editor.innerHTML, "<p><a href=\"about:blank\"><b>abc</b></a><b>X</b></p><p>def</p>",
is(editor.innerHTML, "<p><a href=\"about:blank\"><b>abc</b></a><b>X</b></p><p>def</p>",
"Typing X at end of the <a> element should insert to start of new <b> when caret is moved from the following paragraph");
// I'm not sure whether this behavior should be changed or not, but inconsistent with the case of Backspace from start of <a href>.