Should not request text checking if location is in password field [chromium/src : main]

2 views
Skip to first unread message

Siye Liu (Gerrit)

unread,
Sep 15, 2022, 1:31:25 PMSep 15
to Xiaocheng Hu, blink-...@chromium.org, xiaochen...@chromium.org

Attention is currently required from: Xiaocheng Hu.

Siye Liu would like Xiaocheng Hu to review this change.

View Change

Should not request text checking if location is in password field

In |HotModeSpellCheckRequester::CheckSpellingAt| we have existing logic
to limit text checking if the provided location is not enabled for spell
check and the root editable element has spellcheck attribute set to
false. This logic can allow spellchecking in password field:
<input type="password" />. The root editable element is an editable
element which doesn't have spellcheck attribute set. The if statement is
false and we continue to perform text checking.

We are relying on |CurrentWordIfTypingInPartialWord| to return a partial word to avoid performing spellcheck in password field which is incorrect.

We should instead check if current location is in password field.

Bug: 1363865
Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
---
M third_party/blink/renderer/core/editing/spellcheck/hot_mode_spell_check_requester.cc
1 file changed, 24 insertions(+), 0 deletions(-)

diff --git a/third_party/blink/renderer/core/editing/spellcheck/hot_mode_spell_check_requester.cc b/third_party/blink/renderer/core/editing/spellcheck/hot_mode_spell_check_requester.cc
index 382aabc..ab7edb9 100644
--- a/third_party/blink/renderer/core/editing/spellcheck/hot_mode_spell_check_requester.cc
+++ b/third_party/blink/renderer/core/editing/spellcheck/hot_mode_spell_check_requester.cc
@@ -109,6 +109,8 @@
return;
processed_root_editables_.push_back(root_editable);

+ if (IsInPasswordField(position))
+ return;
if (!root_editable->IsSpellCheckingEnabled() &&
!SpellChecker::IsSpellCheckingEnabledAt(position)) {
return;

To view, visit change 3897474. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
Gerrit-Change-Number: 3897474
Gerrit-PatchSet: 4
Gerrit-Owner: Siye Liu <si...@microsoft.com>
Gerrit-Reviewer: Siye Liu <si...@microsoft.com>
Gerrit-Reviewer: Xiaocheng Hu <xiaoc...@chromium.org>
Gerrit-Attention: Xiaocheng Hu <xiaoc...@chromium.org>
Gerrit-MessageType: newchange

Siye Liu (Gerrit)

unread,
Sep 15, 2022, 1:31:36 PMSep 15
to blink-...@chromium.org, xiaochen...@chromium.org, Xiaocheng Hu, Tricium, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Xiaocheng Hu.

View Change

    To view, visit change 3897474. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
    Gerrit-Change-Number: 3897474
    Gerrit-PatchSet: 4
    Gerrit-Owner: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Attention: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Comment-Date: Thu, 15 Sep 2022 17:31:22 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Siye Liu (Gerrit)

    unread,
    Sep 16, 2022, 6:29:39 PMSep 16
    to blink-...@chromium.org, xiaochen...@chromium.org, Xiaocheng Hu, Tricium, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Xiaocheng Hu.

    View Change

    1 comment:

    • Patchset:

    To view, visit change 3897474. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
    Gerrit-Change-Number: 3897474
    Gerrit-PatchSet: 4
    Gerrit-Owner: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Attention: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Comment-Date: Fri, 16 Sep 2022 22:29:28 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Xiaocheng Hu (Gerrit)

    unread,
    Sep 19, 2022, 1:38:20 PMSep 19
    to Siye Liu, blink-...@chromium.org, xiaochen...@chromium.org, Tricium, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Siye Liu.

    View Change

    1 comment:

    • Patchset:

      • Patch Set #4:

        (Sorry for the delay, OOO last week)

        Is this a real bug? Do we ever create a spell check request for a password field?

        SpellChecker::IsSpellCheckingEnabledAt() already returns false for password fields, so I don't think this patch is needed.

    To view, visit change 3897474. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
    Gerrit-Change-Number: 3897474
    Gerrit-PatchSet: 4
    Gerrit-Owner: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Attention: Siye Liu <si...@microsoft.com>
    Gerrit-Comment-Date: Mon, 19 Sep 2022 17:38:04 +0000

    Siye Liu (Gerrit)

    unread,
    Sep 19, 2022, 1:49:43 PMSep 19
    to blink-...@chromium.org, xiaochen...@chromium.org, Xiaocheng Hu, Tricium, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Xiaocheng Hu.

    View Change

    1 comment:

    • Patchset:

      • Patch Set #4:

        (Sorry for the delay, OOO last week) […]

        Chromium doesn't create spellcheck request for password field. But we do have potential issue because we didn't use correct logic to determine password field.

        |SpellChecker::IsSpellCheckingEnabledAt| does returns false for password fields. However, in |HotModeSpellCheckRequester| https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/editing/spellcheck/hot_mode_spell_check_requester.cc;l=112-113, the whole if statement returns false for password field so we never satisfy the if (|root_editable->IsSpellCheckingEnabled()| return true).

        Since |SpellChecker::IsSpellCheckingEnabledAt| didn't filter out password field as intended, |CurrentWordIfTypingInPartialWord| returns non-null range in password field therefore we won't proceed to create spellcheck request for password field.


        Instead of relying on |CurrentWordIfTypingInPartialWord|, we should use explicit check to filter out password fields.

    To view, visit change 3897474. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
    Gerrit-Change-Number: 3897474
    Gerrit-PatchSet: 4
    Gerrit-Owner: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Attention: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Comment-Date: Mon, 19 Sep 2022 17:49:31 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-MessageType: comment

    Xiaocheng Hu (Gerrit)

    unread,
    Sep 19, 2022, 2:09:08 PMSep 19
    to Siye Liu, blink-...@chromium.org, xiaochen...@chromium.org, Tricium, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Siye Liu.

    View Change

    1 comment:

    To view, visit change 3897474. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
    Gerrit-Change-Number: 3897474
    Gerrit-PatchSet: 4
    Gerrit-Owner: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Attention: Siye Liu <si...@microsoft.com>
    Gerrit-Comment-Date: Mon, 19 Sep 2022 18:08:54 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Xiaocheng Hu <xiaoc...@chromium.org>
    Comment-In-Reply-To: Siye Liu <si...@microsoft.com>
    Gerrit-MessageType: comment

    Xiaocheng Hu (Gerrit)

    unread,
    Sep 19, 2022, 5:56:14 PMSep 19
    to Siye Liu, blink-...@chromium.org, xiaochen...@chromium.org, Tricium, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Siye Liu.

    View Change

    1 comment:

    • Patchset:

      • Patch Set #4:

        I see. […]

        Hi Siye, after looking into it more, I think the real issue is that we didn't properly implement the default spell check behavior [1] for form control elements. So the right place to fix is in Element::IsSpellCheckingEnabled().

        There's another patch working on a pretty much the issue, which I'm going to take over (crrev.com/c/3904367). Can I also take over your patch?

        [1] https://html.spec.whatwg.org/#concept-spellcheck-default

    To view, visit change 3897474. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
    Gerrit-Change-Number: 3897474
    Gerrit-PatchSet: 5
    Gerrit-Owner: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-Attention: Siye Liu <si...@microsoft.com>
    Gerrit-Comment-Date: Mon, 19 Sep 2022 21:56:03 +0000

    Siye Liu (Gerrit)

    unread,
    Sep 19, 2022, 6:04:29 PMSep 19
    to blink-...@chromium.org, xiaochen...@chromium.org, Xiaocheng Hu, Tricium, Chromium LUCI CQ, chromium...@chromium.org

    Siye Liu abandoned this change.

    View Change

    Abandoned Xiaocheng will take over this CL and implement correct default spellcheck behavior for forme control elements.

    To view, visit change 3897474. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iae9af8c3e03a49995ee5dece661345da1a0c5557
    Gerrit-Change-Number: 3897474
    Gerrit-PatchSet: 5
    Gerrit-Owner: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Siye Liu <si...@microsoft.com>
    Gerrit-Reviewer: Xiaocheng Hu <xiaoc...@chromium.org>
    Gerrit-MessageType: abandon
    Reply all
    Reply to author
    Forward
    0 new messages