Bug 1524266 - Should be able to delete non-selectable and non-editable content in a contenteditable subtree. r=mats

This makes our behavior a bit closer to Blink / WebKit.

This patch fixes multiple issues:

First, fixes the caret movement getting stuck on a <select> element inside an
editor. This is because of the IsRootOfAnonymousSubtree() check that I'm
removing. Instead of that, consider NAC unselectable in UsedUserSelect, just
like generated content. This makes us jump across it correctly, and doesn't
regress the test-case that was added in bug 989012.

Second, it allows to select nodes with user-select: none as long as you're on an
editor. This matches WebKit and Blink. It's something you could do earlier
regardless with user-select: all on the parent, which is why the reporter's
test-case worked before my patch. I think being able to jump across these and
delete them on an editor is the right thing to do.

It adds tests for all this plus the same thing working for non-editable contents
(there was no pre-existing test for that).

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

--HG--
extra : moz-landing-system : lando

dom/base/Selection.cpp

@@ -459,20 +459,23 @@ bool Selection::GetInterlinePosition(ErrorResult& aRv) {
   return mFrameSelection->GetHint() == CARET_ASSOCIATE_AFTER;
 }
 
-bool Selection::IsEditorSelection() const {
-  nsINode* focusNode = GetFocusNode();
-  if (!focusNode) {
+static bool IsEditorNode(const nsINode* aNode) {
+  if (!aNode) {
     return false;
   }
 
-  if (focusNode->IsEditable()) {
+  if (aNode->IsEditable()) {
     return true;
   }
 
-  auto* element = Element::FromNode(focusNode);
+  auto* element = Element::FromNode(aNode);
   return element && element->State().HasState(NS_EVENT_STATE_MOZ_READWRITE);
 }
 
+bool Selection::IsEditorSelection() const {
+  return IsEditorNode(GetFocusNode());
+}
+
 Nullable<int16_t> Selection::GetCaretBidiLevel(
     mozilla::ErrorResult& aRv) const {
   if (!mFrameSelection) {
@@ -913,16 +916,16 @@ nsresult Selection::SubtractRange(RangeData* aRange, nsRange* aSubtract,
 
 void Selection::UserSelectRangesToAdd(nsRange* aItem,
                                       nsTArray<RefPtr<nsRange>>& aRangesToAdd) {
-  aItem->ExcludeNonSelectableNodes(&aRangesToAdd);
-  if (aRangesToAdd.IsEmpty()) {
-    ErrorResult err;
-    nsINode* node = aItem->GetStartContainer(err);
-    if (node && node->IsContent() && node->AsContent()->GetEditingHost()) {
-      // A contenteditable node with user-select:none, for example.
-      // Allow it to have a collapsed selection (for the caret).
-      aItem->Collapse(GetDirection() == eDirPrevious);
+  // We cannot directly call IsEditorSelection() because we may be in an
+  // inconsistent state during Collapse() (we're cleared already but we haven't
+  // got a new focus node yet).
+  if (IsEditorNode(aItem->GetStartContainer()) &&
+      IsEditorNode(aItem->GetEndContainer())) {
+    // Don't mess with the selection ranges for editing, editor doesn't really
+    // deal well with multi-range selections.
     aRangesToAdd.AppendElement(aItem);
-    }
+  } else {
+    aItem->ExcludeNonSelectableNodes(&aRangesToAdd);
   }
 }
 

dom/base/test/test_user_select.html

@@ -34,6 +34,7 @@ a { position:absolute; bottom: 0; right:0; }
 <div id="testB"><n><s>aaaa</s>aaa</n>bbbbbbbbccccccc</div>
 <div id="testC">aaaaaaabbbbbbbb<n>cc<s>c</s>cccc</n></div>
 <div id="testE">aaa<s id="testEc1">aaaa<a class="text">bbbb</a>dd<a>cccc</a>ddddddd</s>eeee</div>
+<div id="testI">aaa<span contenteditable="true" spellcheck="false">bbb</span><s>ccc</s>ddd</div>
 <div id="testF">aaaa
 <div class="non-selectable">x</div>
 <div class="non-selectable">x</div>
@@ -245,6 +246,13 @@ function test()
   checkRange(2, [2,0,2,2], e);
   doneTest(e);
 
+  clear();
+  e = document.getElementById('testI');
+  dragSelect(e, 200, 80);
+  checkText('bbd', e);
+  checkRangeCount(2, e);
+  doneTest(e);
+
   // ======================================================
   // ================== shift+click tests =================
   // ======================================================

layout/base/tests/bug1524266-1-ref.html

@@ -0,0 +1,30 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Caret doesn't get stuck in a select element inside an editor</title>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+  div:focus-within {
+    outline: 3px solid blue;
+  }
+  span {
+    outline: none;
+  }
+</style>
+<div>
+  xx
+  <select>
+    <option value="">Placeholder</option>
+  </select>
+  <span contenteditable="true" spellcheck="false">xxx</span>
+</div>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('[contenteditable="true"]').focus();
+  requestAnimationFrame(function() {
+    for (let i = 0; i < 2; ++i)
+      synthesizeKey("KEY_ArrowRight");
+    document.documentElement.removeAttribute("class");
+  });
+});
+</script>

layout/base/tests/bug1524266-1.html

@@ -0,0 +1,27 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Caret doesn't get stuck in a select element inside an editor</title>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+  div:focus {
+    outline: 3px solid blue;
+  }
+</style>
+<div contenteditable="true" spellcheck="false">
+  xx
+  <select>
+    <option value="">Placeholder</option>
+  </select>
+  xxx
+</div>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('[contenteditable="true"]').focus();
+  requestAnimationFrame(function() {
+    for (let i = 0; i < 7; ++i)
+      synthesizeKey("KEY_ArrowRight");
+    document.documentElement.removeAttribute("class");
+  });
+});
+</script>

layout/base/tests/bug1524266-2-ref.html

@@ -0,0 +1,24 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Can delete non-selectable content in an editor</title>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+  div:focus {
+    outline: 3px solid blue;
+  }
+</style>
+<div contenteditable="true" spellcheck="false">
+  xxxxx
+</div>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('[contenteditable="true"]').focus();
+  requestAnimationFrame(function() {
+    // Move after the two x
+    for (let i = 0; i < 2; ++i)
+      synthesizeKey("KEY_ArrowRight");
+    document.documentElement.removeAttribute("class");
+  });
+});
+</script>

layout/base/tests/bug1524266-2.html

@@ -0,0 +1,38 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Can delete non-selectable content in an editor</title>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+  div:focus {
+    outline: 3px solid blue;
+  }
+  /* <select> has user-select: none in the UA sheet, but just in case */
+  select {
+    -moz-user-select: none;
+    user-select: none;
+  }
+</style>
+<div contenteditable="true" spellcheck="false">
+  xx
+  <select>
+    <option value="">Placeholder</option>
+  </select>
+  xxx
+</div>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('[contenteditable="true"]').focus();
+  requestAnimationFrame(function() {
+    // Move after the two x
+    for (let i = 0; i < 2; ++i)
+      synthesizeKey("KEY_ArrowRight");
+    // Select whitespace + <select> + whitespace.
+    for (let i = 0; i < 3; ++i)
+      synthesizeKey("KEY_ArrowRight", { shiftKey: true });
+    // Rip it off.
+    synthesizeKey("KEY_Delete");
+    document.documentElement.removeAttribute("class");
+  });
+});
+</script>

layout/base/tests/bug1524266-3.html

@@ -0,0 +1,37 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Can delete non-selectable content in an editor</title>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+  div:focus {
+    outline: 3px solid blue;
+  }
+  span {
+    -moz-user-select: none;
+    user-select: none;
+  }
+</style>
+<div contenteditable="true" spellcheck="false">
+  xx
+  <span>
+    NOT EDITABLE
+  </span>
+  xxx
+</div>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('[contenteditable="true"]').focus();
+  requestAnimationFrame(function() {
+    // Move after the two x
+    for (let i = 0; i < 2; ++i)
+      synthesizeKey("KEY_ArrowRight");
+    // Select whitespace + <span>
+    for (let i = 0; i < 2; ++i)
+      synthesizeKey("KEY_ArrowRight", { shiftKey: true });
+    // Rip it off.
+    synthesizeKey("KEY_Delete");
+    document.documentElement.removeAttribute("class");
+  });
+});
+</script>

layout/base/tests/bug1524266-4.html

@@ -0,0 +1,33 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Can delete non-editable content in an editor</title>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+  div:focus {
+    outline: 3px solid blue;
+  }
+</style>
+<div contenteditable="true" spellcheck="false">
+  xx
+  <span contenteditable="false">
+    NOT EDITABLE
+  </span>
+  xxx
+</div>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('[contenteditable="true"]').focus();
+  requestAnimationFrame(function() {
+    // Move after the two x
+    for (let i = 0; i < 2; ++i)
+      synthesizeKey("KEY_ArrowRight");
+    // Select whitespace + <span>
+    for (let i = 0; i < 2; ++i)
+      synthesizeKey("KEY_ArrowRight", { shiftKey: true });
+    // Rip it off.
+    synthesizeKey("KEY_Delete");
+    document.documentElement.removeAttribute("class");
+  });
+});
+</script>

layout/base/tests/mochitest.ini

@@ -331,6 +331,12 @@ support-files =
   bug1510942-1-ref.html
   bug1510942-2.html
   bug1510942-2-ref.html
+  bug1524266-1.html
+  bug1524266-1-ref.html
+  bug1524266-2.html
+  bug1524266-2-ref.html
+  bug1524266-3.html
+  bug1524266-4.html
   image_rgrg-256x256.png
   input-invalid-ref.html
   input-maxlength-invalid-change.html

layout/base/tests/test_reftests_with_caret.html

@@ -216,6 +216,7 @@ var tests = [
     [ 'bug1510942-2.html' , 'bug1510942-2-ref.html' ] ,
     [ 'bug1518339-1.html' , 'bug1518339-1-ref.html' ] ,
     [ 'bug1518339-2.html' , 'bug1518339-2-ref.html' ] ,
+    [ 'bug1524266-1.html' , 'bug1524266-1-ref.html' ] ,
     function() {SpecialPowers.pushPrefEnv({'clear': [['layout.accessiblecaret.enabled']]}, nextTest);} ,
 ];
 
@@ -327,6 +328,12 @@ if (navigator.platform.includes("Linux")) {
     // eDirNext, VK_RIGHT / LEFT
     [ 'multi-range-script-select.html#next1SR' , 'multi-range-script-select-ref.html#next1SR'  ] ,
     [ 'multi-range-script-select.html#next1SL' , 'multi-range-script-select-ref.html#next1SL'  ] ,
+
+    // Tries to select and delete non-selectable content in a user-select subtree.
+    [ 'bug1524266-2.html' , 'bug1524266-2-ref.html' ] ,
+    [ 'bug1524266-3.html' , 'bug1524266-2-ref.html' ] ,
+    // Tries to select and delete non-editable content in a user-select subtree.
+    [ 'bug1524266-4.html' , 'bug1524266-2-ref.html' ] ,
   ]);
 }
 

layout/generic/nsFrame.cpp

@@ -3975,8 +3975,20 @@ static bool IsEditingHost(const nsIFrame* aFrame) {
   return element && element->IsEditableRoot();
 }
 
-static StyleUserSelect UsedUserSelect(const nsIFrame* aFrame) {
+static bool IsUnselectable(const nsIFrame* aFrame) {
+  // Pseudo-elements are not selectable.
   if (aFrame->HasAnyStateBits(NS_FRAME_GENERATED_CONTENT)) {
+    return true;
+  }
+
+  // Native anonymous content that is not editable is not selectable either.
+  auto* content = aFrame->GetContent();
+  return content && !content->IsEditable() &&
+         content->IsInNativeAnonymousSubtree();
+}
+
+static StyleUserSelect UsedUserSelect(const nsIFrame* aFrame) {
+  if (IsUnselectable(aFrame)) {
     return StyleUserSelect::None;
   }
 
@@ -8553,11 +8565,7 @@ nsresult nsIFrame::GetFrameFromDirection(
       frameTraversal->Prev();
 
     traversedFrame = frameTraversal->CurrentItem();
-
-    // Skip anonymous elements, but watch out for generated content
-    if (!traversedFrame ||
-        (!traversedFrame->IsGeneratedContentFrame() &&
-         traversedFrame->GetContent()->IsRootOfNativeAnonymousSubtree())) {
+    if (!traversedFrame) {
       return NS_ERROR_FAILURE;
     }