New issue
Advanced search Search tips

Issue 640456 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Pasted text not spell checked when the root editable element of the pasting target starts with invisible node

Project Member Reported by xiaoche...@chromium.org, Aug 24 2016

Issue description

Version: ToT 54.0.2838.0
OS: All

What steps will reproduce the problem?
(1) Open https://jsfiddle.net/ym5k328q/1/
(2) Copy the first line "misssspelllled wordds tto ccopy."
(3) Paste into the first "[]"
(4) Then Paste into the second "[]"

What is the expected output?
After step (4), red markers appear immediately under the misspelled words, just like after step (3).

What do you see instead?
No marker appear after step (4) (unless we do some other action to trigger spell checking again)

Please use labels and text to provide additional information.

 
The root cause is that SpellCheckRequest::invoke performs an unnecessary and sometimes even incorrect check with |canCheckAsynchronously()|, which cancels the spell check request after step (4).

In |canCheckAsynchronously()|, the null pointer check on |range| or |range->firstNode()| is unnecessary, while checking |range->firstNode()->layoutObject()| is incorrect because that the range starts with an invisible node does not mean it's empty.

The check on |toElement(node)->isSpellCheckingEnabled()| should be done in |SpellChecker| instead, because |SpellChecker| typically passes a range that's not just a single node, and the check incorrectly cancels the request if the range passed is something like <div contenteditable spellcheck="false"><span spellcheck="true">foo</span></div>.

This function should be removed.
Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7168dd4011adfe053b7a5c0f308fcf7b1495b31b

commit 7168dd4011adfe053b7a5c0f308fcf7b1495b31b
Author: xiaochengh <xiaochengh@chromium.org>
Date: Wed Aug 24 11:53:13 2016

Stop SpellCheckRequest from checking canCheckAsynchronously()

This patch removes static function canCheckAsynchronously() from
SpellCheckRequester.cpp and stops SpellCheckRequest from calling it.

The removed function is legacy code from the old age before the removal
of synchronous spell checking. In addition, the items checked by this
function do not make much sense now -- either incorrect or redundant.
Hence, the function is removed.

This patch is also a preparation for reducing the use of
TextCheckingParagraph: http://crrev.com/2273453003

BUG= 640456 
TEST=editing/spelling/paste_into_element_starting_with_invisible.html

Review-Url: https://codereview.chromium.org/2248413003
Cr-Commit-Position: refs/heads/master@{#414049}

[add] https://crrev.com/7168dd4011adfe053b7a5c0f308fcf7b1495b31b/third_party/WebKit/LayoutTests/editing/spelling/paste_into_element_starting_with_invisible.html
[modify] https://crrev.com/7168dd4011adfe053b7a5c0f308fcf7b1495b31b/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckRequester.cpp
[modify] https://crrev.com/7168dd4011adfe053b7a5c0f308fcf7b1495b31b/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
[modify] https://crrev.com/7168dd4011adfe053b7a5c0f308fcf7b1495b31b/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.h

Status: Fixed (was: Assigned)
Components: -UI>Browser>Spellcheck UI>Browser>Language>Spellcheck

Sign in to add a comment