[Spellchecker] Handles nested editables correctly in cold mode checker

When the currently focused editable element is nested in an uneditable element (see the new layout test in this CL as an example), cold mode spellchecker may create invalid checking ranges: - the range starts at the end position of the last sentence - the range ends at the last position in the focused editable element - when the editable element is nested in uneditable element, a reversed range is created This patch fixes the issue by using the highest root editable as the element to check, so that all sentences are within the element. To ensure that uneditable nodes in the highest root are not marked, this patch also modifies SpellChecker::IsSpellCheckingEnabledAt() to return false on uneditable nodes. Bug: 848026 Change-Id: I26d9852ffc66aad150cf3856fd402fc31e3ad3a0 Reviewed-on: https://chromium-review.googlesource.com/1080215 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#563159}
qweri0p · May 31, 2018 · 11d5f36 · 11d5f36
1 parent e88aa42
commit 11d5f36
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 3 deletions.
diff --git a/third_party/WebKit/LayoutTests/editing/spelling/cold_mode_nested_editables.html b/third_party/WebKit/LayoutTests/editing/spelling/cold_mode_nested_editables.html
@@ -0,0 +1,31 @@
+<!doctype html>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../assert_selection.js"></script>
+<script src="spellcheck_test.js"></script>
+<script>
+const longCorrectText = ' word '.repeat(20);
+
+// crbug.com/848026
+spellcheck_test(
+    [
+      `<div contenteditable>${longCorrectText}`,
+      '<span contenteditable="false">zz ',
+      '<span contenteditable id="target">zz</span>',
+      ' zz</span>',
+      `${longCorrectText}</div>`
+    ].join(''),
+    document => document.getElementById('target').focus(),
+    [
+      `<div contenteditable>${longCorrectText}`,
+      '<span contenteditable="false">zz ',
+      '<span contenteditable id="target">#zz#</span>',
+      ' zz</span>',
+      `${longCorrectText}</div>`
+    ].join(''),
+    {
+      title: 'Cold mode checks text in nested editables correctly without crashing',
+      needsFullCheck: true
+    }
+);
+</script>
diff --git a/third_party/blink/renderer/core/editing/spellcheck/cold_mode_spell_check_requester.cc b/third_party/blink/renderer/core/editing/spellcheck/cold_mode_spell_check_requester.cc
@@ -63,10 +63,11 @@ const Element* ColdModeSpellCheckRequester::CurrentFocusedEditable() const {
   if (position.IsNull())
     return nullptr;
 
-  const Element* element = RootEditableElementOf(position);
-  if (!element || !element->isConnected())
+  const ContainerNode* root = HighestEditableRoot(position);
+  if (!root || !root->isConnected() || !root->IsElementNode())
     return nullptr;
 
+  const Element* element = ToElement(root);
   if (!element->IsSpellCheckingEnabled() ||
       !SpellChecker::IsSpellCheckingEnabledAt(position))
     return nullptr;

diff --git a/third_party/blink/renderer/core/editing/spellcheck/spell_checker.cc b/third_party/blink/renderer/core/editing/spellcheck/spell_checker.cc
@@ -783,7 +783,8 @@ bool SpellChecker::IsSpellCheckingEnabledAt(const Position& position) {
   }
   HTMLElement* element =
       Traversal<HTMLElement>::FirstAncestorOrSelf(*position.AnchorNode());
-  return element && element->IsSpellCheckingEnabled();
+  return element && element->IsSpellCheckingEnabled() &&
+         HasEditableStyle(*element);
 }
 
 STATIC_ASSERT_ENUM(kWebTextDecorationTypeSpelling, kTextDecorationTypeSpelling);