Remove an incorrect heuristic from spellchecker

Spellchecker has a heuristic that, if the bounding box of an editing host is not in viewport, don't check anything in the editing host. This is wrong as there might still be descendants of the editing host inside the viewport. Hence, this patch removes the heuristic. Bug: 768231 Change-Id: I22995b365c6a3e10eedb76a4868746f09261e733 Reviewed-on: https://chromium-review.googlesource.com/903265 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#534848}

Remove an incorrect heuristic from spellchecker
Spellchecker has a heuristic that, if the bounding box of an editing host is not in viewport, don't check anything in the editing host. This is wrong as there might still be descendants of the editing host inside the viewport. Hence, this patch removes the heuristic. Bug: 768231 Change-Id: I22995b365c6a3e10eedb76a4868746f09261e733 Reviewed-on: https://chromium-review.googlesource.com/903265 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#534848}
a91c371d · Xiaocheng Hu · Commit Bot · c318f63a · a91c371d · a91c371d
Commit a91c371d authored Feb 07, 2018 by Xiaocheng Hu Committed by Commit Bot Feb 07, 2018
2 changed files
--- a/third_party/WebKit/LayoutTests/editing/spelling/editing_host_out_of_viewport.html
+++ b/third_party/WebKit/LayoutTests/editing/spelling/editing_host_out_of_viewport.html
+<!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>
+// It's possible that the bounding box of an editing host is out of viewport,
+// but still there is visible content in it. Verify that spelling is checked
+// in this case.
+
+const longContent = '<p>line.</p>'.repeat(50) + 'zz|.';
+const expectedText = '<p>line.</p>'.repeat(50) + '#zz#.';
+
+spellcheck_test(
+  `<div contenteditable style="height: 10px">${longContent}</div>`,
+  document => {
+    const frameWindow = document.defaultView;
+    const frameViewport = frameWindow.visualViewport;
+    frameWindow.scrollBy(0, frameViewport.height);
+  },
+  `<div contenteditable style="height: 10px">${expectedText}</div>`,
+  'Spelling is checked even if editing host is out of viewport');
+</script>
--- a/third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp
+++ b/third_party/WebKit/Source/core/editing/spellcheck/HotModeSpellCheckRequester.cpp
@@ -57,17 +57,6 @@ EphemeralRange CurrentWordIfTypingInPartialWord(const Element& editable) {
  return AdjacentWordIfExists(selection.Base());
 }

-bool IsUnderActiveEditing(const Element& editable, const Position& position) {
-  if (!editable.IsSpellCheckingEnabled() &&
-      !SpellChecker::IsSpellCheckingEnabledAt(position))
-    return false;
-  if (editable.VisibleBoundsInVisualViewport().IsEmpty())
-    return false;
-  // TODO(xiaochengh): Design more aggressive strategies to reduce checking when
-  // we are just moving selection around without modifying anything.
-  return true;
-}
-
 EphemeralRange CalculateHotModeCheckingRange(const Element& editable,
                                             const Position& position) {
  // Check everything in |editable| if its total length is short.
@@ -118,8 +107,10 @@ void HotModeSpellCheckRequester::CheckSpellingAt(const Position& position) {
    return;
  processed_root_editables_.push_back(root_editable);

-  if (!IsUnderActiveEditing(*root_editable, position))
+  if (!root_editable->IsSpellCheckingEnabled() &&
+      !SpellChecker::IsSpellCheckingEnabledAt(position)) {
    return;
+  }

  const EphemeralRange& current_word =
      CurrentWordIfTypingInPartialWord(*root_editable);