[PEPC] Disable focusing via script [chromium/src : main]

0 views
Skip to first unread message

Blink W3C Test Autoroller (Gerrit)

unread,
Apr 17, 2024, 4:02:45 PMApr 17
to Andy Paicu, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org

Message from Blink W3C Test Autoroller

Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/45766.

When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.

WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
Gerrit-Change-Number: 5463501
Gerrit-PatchSet: 1
Gerrit-Owner: Andy Paicu <andy...@chromium.org>
Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
Gerrit-Comment-Date: Wed, 17 Apr 2024 20:02:33 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Andy Paicu (Gerrit)

unread,
Apr 18, 2024, 9:34:53 AMApr 18
to Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
Attention needed from Thomas Nguyen

Andy Paicu added 1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Andy Paicu . resolved

Hi Thomas, PTAL

Open in Gerrit

Related details

Attention is currently required from:
  • Thomas Nguyen
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
Gerrit-Change-Number: 5463501
Gerrit-PatchSet: 2
Gerrit-Owner: Andy Paicu <andy...@chromium.org>
Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
Gerrit-Attention: Thomas Nguyen <tun...@chromium.org>
Gerrit-Comment-Date: Thu, 18 Apr 2024 13:34:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Thomas Nguyen (Gerrit)

unread,
Apr 18, 2024, 9:39:57 AMApr 18
to Andy Paicu, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
Attention needed from Andy Paicu

Thomas Nguyen added 1 comment

File third_party/blink/renderer/core/html/html_permission_element.cc
Line 335, Patchset 2 (Latest): if (params.focus_trigger == FocusTrigger::kScript) {
return;
}
Thomas Nguyen . unresolved

A bit overlap as I am also looking at this, but thanks for the CL.

From what I saw, the `kScript` is called even not for focus() from JS, for example focus restoring. What will be expect in this case, we might need to verify it?

Open in Gerrit

Related details

Attention is currently required from:
  • Andy Paicu
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
    Gerrit-Change-Number: 5463501
    Gerrit-PatchSet: 2
    Gerrit-Owner: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-Attention: Andy Paicu <andy...@chromium.org>
    Gerrit-Comment-Date: Thu, 18 Apr 2024 13:39:48 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Andy Paicu (Gerrit)

    unread,
    Apr 18, 2024, 10:15:16 AMApr 18
    to Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
    Attention needed from Thomas Nguyen

    Andy Paicu added 1 comment

    File third_party/blink/renderer/core/html/html_permission_element.cc
    Line 335, Patchset 2 (Latest): if (params.focus_trigger == FocusTrigger::kScript) {
    return;
    }
    Thomas Nguyen . unresolved

    A bit overlap as I am also looking at this, but thanks for the CL.

    From what I saw, the `kScript` is called even not for focus() from JS, for example focus restoring. What will be expect in this case, we might need to verify it?

    Andy Paicu

    Thanks for bringing this to my attention, I had a look through usages:
    [1] `Element::focusForBindings` this is for the `focus()` function so we for sure want to avoid that.
    [2] `WebElement::SelectText` this seems to be for focusing text controls that have had their entire contents selected. Should be irrelevant to us.
    [3] `TextControlElement::select()` seems to complement the one above
    [4], [5] `HTMLDialogElement::close`, `HTMLElement::HidePopoverInternal` this is related to modal elements being closed and restoring focus to whatever was focused before they opened. If the permission element was focused before the dialog/popup opened, there is a good chance that the user has forgotten already about it and any enter/space key presses are probably unintentional. I think in this case ignoring the focus event is the correct thing to do.

    [1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc
    [2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/exported/web_element.cc;l=189
    [3] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/text_control_element.cc;l=277
    [4] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_dialog_element.cc;l=187
    [5] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_element.cc;l=1861

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Thomas Nguyen
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
    Gerrit-Change-Number: 5463501
    Gerrit-PatchSet: 2
    Gerrit-Owner: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-Attention: Thomas Nguyen <tun...@chromium.org>
    Gerrit-Comment-Date: Thu, 18 Apr 2024 14:15:05 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Thomas Nguyen <tun...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Thomas Nguyen (Gerrit)

    unread,
    Apr 19, 2024, 8:38:51 AMApr 19
    to Andy Paicu, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
    Attention needed from Andy Paicu

    Thomas Nguyen added 1 comment

    File third_party/blink/renderer/core/html/html_permission_element.cc
    Line 335, Patchset 2 (Latest): if (params.focus_trigger == FocusTrigger::kScript) {
    return;
    }
    Thomas Nguyen . unresolved

    A bit overlap as I am also looking at this, but thanks for the CL.

    From what I saw, the `kScript` is called even not for focus() from JS, for example focus restoring. What will be expect in this case, we might need to verify it?

    Andy Paicu

    Thanks for bringing this to my attention, I had a look through usages:
    [1] `Element::focusForBindings` this is for the `focus()` function so we for sure want to avoid that.
    [2] `WebElement::SelectText` this seems to be for focusing text controls that have had their entire contents selected. Should be irrelevant to us.
    [3] `TextControlElement::select()` seems to complement the one above
    [4], [5] `HTMLDialogElement::close`, `HTMLElement::HidePopoverInternal` this is related to modal elements being closed and restoring focus to whatever was focused before they opened. If the permission element was focused before the dialog/popup opened, there is a good chance that the user has forgotten already about it and any enter/space key presses are probably unintentional. I think in this case ignoring the focus event is the correct thing to do.

    [1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc
    [2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/exported/web_element.cc;l=189
    [3] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/text_control_element.cc;l=277
    [4] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_dialog_element.cc;l=187
    [5] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_element.cc;l=1861

    Thomas Nguyen

    Thank you. And I think, it would be narrowed down to use mojom::blink::FocusType::kScript instead of FocusTrigger::kScript.
    FocusTrigger::kScript is set by default and there would be more callees that we are note aware of.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Andy Paicu
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
    Gerrit-Change-Number: 5463501
    Gerrit-PatchSet: 2
    Gerrit-Owner: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-Attention: Andy Paicu <andy...@chromium.org>
    Gerrit-Comment-Date: Fri, 19 Apr 2024 12:38:43 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Andy Paicu <andy...@chromium.org>
    Comment-In-Reply-To: Thomas Nguyen <tun...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Andy Paicu (Gerrit)

    unread,
    Apr 23, 2024, 8:31:10 AMApr 23
    to Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
    Attention needed from Thomas Nguyen

    Andy Paicu added 1 comment

    File third_party/blink/renderer/core/html/html_permission_element.cc
    Line 335, Patchset 2: if (params.focus_trigger == FocusTrigger::kScript) {
    return;
    }
    Thomas Nguyen . resolved

    A bit overlap as I am also looking at this, but thanks for the CL.

    From what I saw, the `kScript` is called even not for focus() from JS, for example focus restoring. What will be expect in this case, we might need to verify it?

    Andy Paicu

    Thanks for bringing this to my attention, I had a look through usages:
    [1] `Element::focusForBindings` this is for the `focus()` function so we for sure want to avoid that.
    [2] `WebElement::SelectText` this seems to be for focusing text controls that have had their entire contents selected. Should be irrelevant to us.
    [3] `TextControlElement::select()` seems to complement the one above
    [4], [5] `HTMLDialogElement::close`, `HTMLElement::HidePopoverInternal` this is related to modal elements being closed and restoring focus to whatever was focused before they opened. If the permission element was focused before the dialog/popup opened, there is a good chance that the user has forgotten already about it and any enter/space key presses are probably unintentional. I think in this case ignoring the focus event is the correct thing to do.

    [1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc
    [2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/exported/web_element.cc;l=189
    [3] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/text_control_element.cc;l=277
    [4] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_dialog_element.cc;l=187
    [5] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_element.cc;l=1861

    Thomas Nguyen

    Thank you. And I think, it would be narrowed down to use mojom::blink::FocusType::kScript instead of FocusTrigger::kScript.
    FocusTrigger::kScript is set by default and there would be more callees that we are note aware of.

    Andy Paicu

    Ah, I see what you mean now, yes you are right. Done.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Thomas Nguyen
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
    Gerrit-Change-Number: 5463501
    Gerrit-PatchSet: 3
    Gerrit-Owner: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-Attention: Thomas Nguyen <tun...@chromium.org>
    Gerrit-Comment-Date: Tue, 23 Apr 2024 12:30:57 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Thomas Nguyen (Gerrit)

    unread,
    Apr 26, 2024, 6:52:37 AMApr 26
    to Andy Paicu, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
    Attention needed from Andy Paicu

    Thomas Nguyen voted Code-Review+1

    Code-Review+1
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Andy Paicu
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
    Gerrit-Change-Number: 5463501
    Gerrit-PatchSet: 5
    Gerrit-Owner: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-Attention: Andy Paicu <andy...@chromium.org>
    Gerrit-Comment-Date: Fri, 26 Apr 2024 10:52:27 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    open
    diffy

    Andy Paicu (Gerrit)

    unread,
    Apr 26, 2024, 7:01:31 AMApr 26
    to Mason Freed, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
    Attention needed from Mason Freed

    Andy Paicu added 1 comment

    Patchset-level comments
    File-level comment, Patchset 5 (Latest):
    Andy Paicu . resolved

    Hi Mason, PTAL html_permission_element.*

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mason Freed
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
    Gerrit-Change-Number: 5463501
    Gerrit-PatchSet: 5
    Gerrit-Owner: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-Attention: Mason Freed <mas...@chromium.org>
    Gerrit-Comment-Date: Fri, 26 Apr 2024 11:01:20 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    open
    diffy

    Andy Paicu (Gerrit)

    unread,
    Apr 26, 2024, 7:02:02 AMApr 26
    to Mason Freed, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
    Attention needed from Mason Freed

    Andy Paicu voted Auto-Submit+1

    Auto-Submit+1
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mason Freed
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
    Gerrit-Change-Number: 5463501
    Gerrit-PatchSet: 6
    Gerrit-Owner: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-Attention: Mason Freed <mas...@chromium.org>
    Gerrit-Comment-Date: Fri, 26 Apr 2024 11:01:49 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    open
    diffy

    Mason Freed (Gerrit)

    unread,
    Apr 29, 2024, 3:26:59 PMApr 29
    to Andy Paicu, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
    Attention needed from Andy Paicu

    Mason Freed added 4 comments

    Commit Message
    Line 9, Patchset 6 (Latest):The PEPC element should not be able to focused via the `focus()`
    function since it can be abused for obtaining unintentional clicks by
    focusing the element at unexpected times.
    Mason Freed . unresolved

    So this is quite unique/odd behavior that I don't believe has a precedent. I'm worried about accessibility and other things getting broken. Is there a discussion somewhere about this behavior?

    File chrome/browser/ui/views/permissions/embedded_permission_prompt_interactive_uitest.cc
    Line 392, Patchset 6 (Latest): console.log(event.target.id);
    Mason Freed . unresolved

    Probably don't want a console.log in here?

    Line 404, Patchset 6 (Latest): // The exact number of "tab" presses needed to pass through all elements
    Mason Freed . unresolved

    Why is that? You have `tabindex=0` on the important ones, so I would think the sequential focus behavior would be the same.

    File third_party/blink/renderer/core/html/html_permission_element.cc
    Line 355, Patchset 6 (Latest): if (params.type == mojom::blink::FocusType::kScript) {
    return;
    }
    Mason Freed . unresolved

    So you don't want to do this, since it'll break accessibility. (The a11y code will think the element is focusable, and it won't be.)

    You should instead override `Element::IsFocusable()` appropriately.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Andy Paicu
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
      Gerrit-Change-Number: 5463501
      Gerrit-PatchSet: 6
      Gerrit-Owner: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
      Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
      Gerrit-Attention: Andy Paicu <andy...@chromium.org>
      Gerrit-Comment-Date: Mon, 29 Apr 2024 19:26:48 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Andy Paicu (Gerrit)

      unread,
      Apr 30, 2024, 5:07:47 AMApr 30
      to Mason Freed, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
      Attention needed from Mason Freed

      Andy Paicu added 4 comments

      Commit Message
      Line 9, Patchset 6 (Latest):The PEPC element should not be able to focused via the `focus()`
      function since it can be abused for obtaining unintentional clicks by
      focusing the element at unexpected times.
      Mason Freed . unresolved

      So this is quite unique/odd behavior that I don't believe has a precedent. I'm worried about accessibility and other things getting broken. Is there a discussion somewhere about this behavior?

      Andy Paicu

      The desire is to prevent programmatic focus on the permission element as it could easily be used by bad developers to obtain unintended activations (by getting the user to hit enter/space when the developers has sneakily focused the permission element). So we want to prevent APIs like `focus()` (I believe right now the only APIs to do this are `focus()` and `blur()`).

      The reasoning is similar to why we don't want to allow developers to use `click()` and we filter out those events.

      File chrome/browser/ui/views/permissions/embedded_permission_prompt_interactive_uitest.cc
      Mason Freed . resolved

      Probably don't want a console.log in here?

      Andy Paicu

      Oops.

      Line 404, Patchset 6 (Latest): // The exact number of "tab" presses needed to pass through all elements
      Mason Freed . resolved

      Why is that? You have `tabindex=0` on the important ones, so I would think the sequential focus behavior would be the same.

      Andy Paicu

      The variations seem to be on how many tabs it takes to actually reach the web contents elements, as it will first tab through the browser UI elements. I can re-work

      File third_party/blink/renderer/core/html/html_permission_element.cc
      Line 355, Patchset 6 (Latest): if (params.type == mojom::blink::FocusType::kScript) {
      return;
      }
      Mason Freed . unresolved

      So you don't want to do this, since it'll break accessibility. (The a11y code will think the element is focusable, and it won't be.)

      You should instead override `Element::IsFocusable()` appropriately.

      Andy Paicu

      `Element::IsFocusable()` doesn't seem to provide the information we need as the `UpdateBehavior` enum doesn't have a value that maps to "focus events coming from a script". I don't know how I would override `Element::IsFocusable()` so that it keeps the same behavior.

      I'm confused why this would negatively impact A11y. The focus type enum specifically mentions accessibility events will be `kNone` and we're only filtering out `kScript`: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/input/focus_type.mojom;l=7

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Mason Freed
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
      Gerrit-Change-Number: 5463501
      Gerrit-PatchSet: 6
      Gerrit-Owner: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
      Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
      Gerrit-Attention: Mason Freed <mas...@chromium.org>
      Gerrit-Comment-Date: Tue, 30 Apr 2024 09:07:32 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Andy Paicu (Gerrit)

      unread,
      Apr 30, 2024, 5:12:56 AMApr 30
      to Mason Freed, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
      Attention needed from Mason Freed

      Andy Paicu added 1 comment

      File chrome/browser/ui/views/permissions/embedded_permission_prompt_interactive_uitest.cc
      Line 404, Patchset 6 (Latest): // The exact number of "tab" presses needed to pass through all elements
      Mason Freed . resolved

      Why is that? You have `tabindex=0` on the important ones, so I would think the sequential focus behavior would be the same.

      Andy Paicu

      The variations seem to be on how many tabs it takes to actually reach the web contents elements, as it will first tab through the browser UI elements. I can re-work

      Andy Paicu

      Somehow my previous comments got cut out:

      I can re-work it to something like:

      • tab until the first element gets focus
      • tab 4 more times
      • check all elements had focus

      if you think this approach is too shoddy.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Mason Freed
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
      Gerrit-Change-Number: 5463501
      Gerrit-PatchSet: 6
      Gerrit-Owner: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
      Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
      Gerrit-Attention: Mason Freed <mas...@chromium.org>
      Gerrit-Comment-Date: Tue, 30 Apr 2024 09:12:41 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Andy Paicu <andy...@chromium.org>
      Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Mason Freed (Gerrit)

      unread,
      Apr 30, 2024, 1:56:48 PMApr 30
      to Andy Paicu, Aaron Leventhal, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
      Attention needed from Aaron Leventhal and Andy Paicu

      Mason Freed added 5 comments

      Patchset-level comments
      File-level comment, Patchset 7 (Latest):
      Mason Freed . resolved

      +aleventhal - please see the question below

      Commit Message
      Line 9, Patchset 6:The PEPC element should not be able to focused via the `focus()`

      function since it can be abused for obtaining unintentional clicks by
      focusing the element at unexpected times.
      Mason Freed . resolved

      So this is quite unique/odd behavior that I don't believe has a precedent. I'm worried about accessibility and other things getting broken. Is there a discussion somewhere about this behavior?

      Andy Paicu

      The desire is to prevent programmatic focus on the permission element as it could easily be used by bad developers to obtain unintended activations (by getting the user to hit enter/space when the developers has sneakily focused the permission element). So we want to prevent APIs like `focus()` (I believe right now the only APIs to do this are `focus()` and `blur()`).

      The reasoning is similar to why we don't want to allow developers to use `click()` and we filter out those events.

      Mason Freed

      This will all be interesting to get into standards. But I understand the desire here at least.

      File third_party/blink/renderer/core/html/html_permission_element.h
      Line 52, Patchset 7 (Latest): void Focus(const FocusParams& params) override;
      Mason Freed . unresolved

      One thing I do think is absolutely required is to override `SupportsFocus()` here to be `true`, since you are explicitly allowing the element to be focused. Right?

      File third_party/blink/renderer/core/html/html_permission_element.cc
      Line 355, Patchset 6: if (params.type == mojom::blink::FocusType::kScript) {
      return;
      }
      Mason Freed . unresolved

      So you don't want to do this, since it'll break accessibility. (The a11y code will think the element is focusable, and it won't be.)

      You should instead override `Element::IsFocusable()` appropriately.

      Andy Paicu

      `Element::IsFocusable()` doesn't seem to provide the information we need as the `UpdateBehavior` enum doesn't have a value that maps to "focus events coming from a script". I don't know how I would override `Element::IsFocusable()` so that it keeps the same behavior.

      I'm confused why this would negatively impact A11y. The focus type enum specifically mentions accessibility events will be `kNone` and we're only filtering out `kScript`: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/input/focus_type.mojom;l=7

      Mason Freed

      Right - `IsFocusable()` is a property of the node, so it can't change depending on "how" the node tries to become focused. I suppose this might work?

      +aleventhal to comment on potential issues around not allowing script focus. Perhaps there aren't any issues!

      File third_party/blink/web_tests/external/wpt/html/semantics/permission-element/no-focus.tentative.html
      Line 15, Patchset 7 (Latest): let el = document.getElementById("id1");
      el.focus();
      assert_not_equals(document.activeElement, el,
      "Permission element should not be focused");
      Mason Freed . unresolved

      Perhaps add a test that uses `test_driver`'s `pointerDown()` to manually click the element and make sure it *does* get focus in that case?

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Aaron Leventhal
      • Andy Paicu
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
      Gerrit-Change-Number: 5463501
      Gerrit-PatchSet: 7
      Gerrit-Owner: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Thomas Nguyen <tun...@google.com>
      Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
      Gerrit-Attention: Andy Paicu <andy...@chromium.org>
      Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Comment-Date: Tue, 30 Apr 2024 17:56:35 +0000
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Andy Paicu (Gerrit)

      unread,
      May 6, 2024, 11:09:18 AM (12 days ago) May 6
      to Aaron Leventhal, Mason Freed, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
      Attention needed from Aaron Leventhal and Mason Freed

      Andy Paicu voted and added 2 comments

      Votes added by Andy Paicu

      Auto-Submit+0
      Commit-Queue+1

      2 comments

      File third_party/blink/renderer/core/html/html_permission_element.h
      Line 52, Patchset 7: void Focus(const FocusParams& params) override;
      Mason Freed . resolved

      One thing I do think is absolutely required is to override `SupportsFocus()` here to be `true`, since you are explicitly allowing the element to be focused. Right?

      Andy Paicu

      Done

      File third_party/blink/web_tests/external/wpt/html/semantics/permission-element/no-focus.tentative.html
      Line 15, Patchset 7: let el = document.getElementById("id1");

      el.focus();
      assert_not_equals(document.activeElement, el,
      "Permission element should not be focused");
      Mason Freed . resolved

      Perhaps add a test that uses `test_driver`'s `pointerDown()` to manually click the element and make sure it *does* get focus in that case?

      Andy Paicu

      Done

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Aaron Leventhal
      • Mason Freed
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
      Gerrit-Change-Number: 5463501
      Gerrit-PatchSet: 8
      Gerrit-Owner: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
      Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
      Gerrit-Attention: Mason Freed <mas...@chromium.org>
      Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Comment-Date: Mon, 06 May 2024 15:09:06 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Aaron Leventhal (Gerrit)

      unread,
      May 6, 2024, 12:00:30 PM (12 days ago) May 6
      to Andy Paicu, Mason Freed, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
      Attention needed from Andy Paicu and Mason Freed

      Aaron Leventhal added 1 comment

      File third_party/blink/renderer/core/html/html_permission_element.cc
      Line 355, Patchset 6: if (params.type == mojom::blink::FocusType::kScript) {
      return;
      }
      Mason Freed . unresolved

      So you don't want to do this, since it'll break accessibility. (The a11y code will think the element is focusable, and it won't be.)

      You should instead override `Element::IsFocusable()` appropriately.

      Andy Paicu

      `Element::IsFocusable()` doesn't seem to provide the information we need as the `UpdateBehavior` enum doesn't have a value that maps to "focus events coming from a script". I don't know how I would override `Element::IsFocusable()` so that it keeps the same behavior.

      I'm confused why this would negatively impact A11y. The focus type enum specifically mentions accessibility events will be `kNone` and we're only filtering out `kScript`: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/input/focus_type.mojom;l=7

      Mason Freed

      Right - `IsFocusable()` is a property of the node, so it can't change depending on "how" the node tries to become focused. I suppose this might work?

      +aleventhal to comment on potential issues around not allowing script focus. Perhaps there aren't any issues!

      Aaron Leventhal

      Can you tab to the element?
      If so, I would think screen reader users will need to be able to navigate to the element in their virtual buffer, and set focus to it.
      If that's true, it needs to support the focusable state.

      Either way, we should have a content_browsertest for this element that asserts the correct focusable state, and correct behavior if the Action::kFocus action is used on it. Probably would go into content/browser/accessibility/accessibility_action_browsertest.cc.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Andy Paicu
      • Mason Freed
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
      Gerrit-Change-Number: 5463501
      Gerrit-PatchSet: 8
      Gerrit-Owner: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
      Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
      Gerrit-Attention: Andy Paicu <andy...@chromium.org>
      Gerrit-Attention: Mason Freed <mas...@chromium.org>
      Gerrit-Comment-Date: Mon, 06 May 2024 16:00:14 +0000
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Mason Freed (Gerrit)

      unread,
      May 6, 2024, 1:39:32 PM (12 days ago) May 6
      to Andy Paicu, Aaron Leventhal, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
      Attention needed from Aaron Leventhal and Andy Paicu

      Mason Freed added 1 comment

      File third_party/blink/renderer/core/html/html_permission_element.cc
      Line 355, Patchset 6: if (params.type == mojom::blink::FocusType::kScript) {
      return;
      }
      Mason Freed . unresolved

      So you don't want to do this, since it'll break accessibility. (The a11y code will think the element is focusable, and it won't be.)

      You should instead override `Element::IsFocusable()` appropriately.

      Andy Paicu

      `Element::IsFocusable()` doesn't seem to provide the information we need as the `UpdateBehavior` enum doesn't have a value that maps to "focus events coming from a script". I don't know how I would override `Element::IsFocusable()` so that it keeps the same behavior.

      I'm confused why this would negatively impact A11y. The focus type enum specifically mentions accessibility events will be `kNone` and we're only filtering out `kScript`: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/input/focus_type.mojom;l=7

      Mason Freed

      Right - `IsFocusable()` is a property of the node, so it can't change depending on "how" the node tries to become focused. I suppose this might work?

      +aleventhal to comment on potential issues around not allowing script focus. Perhaps there aren't any issues!

      Aaron Leventhal

      Can you tab to the element?
      If so, I would think screen reader users will need to be able to navigate to the element in their virtual buffer, and set focus to it.
      If that's true, it needs to support the focusable state.

      Either way, we should have a content_browsertest for this element that asserts the correct focusable state, and correct behavior if the Action::kFocus action is used on it. Probably would go into content/browser/accessibility/accessibility_action_browsertest.cc.

      Mason Freed

      Aaron, when an AT using a virtual buffer this way tries to focus an element, what value of `mojom::blink::FocusType` does it use? I don't see one that is like `kAccessibility`, so I'm concerned that perhaps those calls use `kScript` which will get blocked here.

      I presume this is why you're asking for an a11y test for this behavior, so perhaps that'll answer my question.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Aaron Leventhal
      • Andy Paicu
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
      Gerrit-Change-Number: 5463501
      Gerrit-PatchSet: 8
      Gerrit-Owner: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
      Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
      Gerrit-Attention: Andy Paicu <andy...@chromium.org>
      Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Comment-Date: Mon, 06 May 2024 17:39:18 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Andy Paicu <andy...@chromium.org>
      Comment-In-Reply-To: Aaron Leventhal <aleve...@chromium.org>
      Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Andy Paicu (Gerrit)

      unread,
      May 6, 2024, 2:28:37 PM (12 days ago) May 6
      to Nektarios Paisios, Kevin Babbitt, (Julie)Jeongeun Kim, AyeAye, Aaron Leventhal, Mason Freed, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, nektar...@chromium.org, josiah...@chromium.org, aleventhal...@chromium.org, dtseng...@chromium.org, blink-rev...@chromium.org, kyungjunle...@google.com, hirokisa...@chromium.org, yuzo+...@chromium.org, abigailbk...@google.com, francisjp...@google.com, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
      Attention needed from Aaron Leventhal, Mason Freed and Thomas Nguyen

      Andy Paicu added 1 comment

      File third_party/blink/renderer/core/html/html_permission_element.cc
      Line 355, Patchset 6: if (params.type == mojom::blink::FocusType::kScript) {
      return;
      }
      Mason Freed . unresolved

      So you don't want to do this, since it'll break accessibility. (The a11y code will think the element is focusable, and it won't be.)

      You should instead override `Element::IsFocusable()` appropriately.

      Andy Paicu

      `Element::IsFocusable()` doesn't seem to provide the information we need as the `UpdateBehavior` enum doesn't have a value that maps to "focus events coming from a script". I don't know how I would override `Element::IsFocusable()` so that it keeps the same behavior.

      I'm confused why this would negatively impact A11y. The focus type enum specifically mentions accessibility events will be `kNone` and we're only filtering out `kScript`: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/input/focus_type.mojom;l=7

      Mason Freed

      Right - `IsFocusable()` is a property of the node, so it can't change depending on "how" the node tries to become focused. I suppose this might work?

      +aleventhal to comment on potential issues around not allowing script focus. Perhaps there aren't any issues!

      Aaron Leventhal

      Can you tab to the element?
      If so, I would think screen reader users will need to be able to navigate to the element in their virtual buffer, and set focus to it.
      If that's true, it needs to support the focusable state.

      Either way, we should have a content_browsertest for this element that asserts the correct focusable state, and correct behavior if the Action::kFocus action is used on it. Probably would go into content/browser/accessibility/accessibility_action_browsertest.cc.

      Mason Freed

      Aaron, when an AT using a virtual buffer this way tries to focus an element, what value of `mojom::blink::FocusType` does it use? I don't see one that is like `kAccessibility`, so I'm concerned that perhaps those calls use `kScript` which will get blocked here.

      I presume this is why you're asking for an a11y test for this behavior, so perhaps that'll answer my question.

      Andy Paicu

      As per the comment in [1], this shouldn't apply to accessibility events. I've added a test and `SupportsFocus` implementation. It's also brought to attention the fact that the permission element doesn't have its default role set to "button" so it seemed fitting to do it here. PTAL.

      [1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/input/focus_type.mojom;l=7

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Aaron Leventhal
      • Mason Freed
      • Thomas Nguyen
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I1fe10d3315120f03accfc99b518bef971ecd2947
      Gerrit-Change-Number: 5463501
      Gerrit-PatchSet: 8
      Gerrit-Owner: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Reviewer: Andy Paicu <andy...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Thomas Nguyen <tun...@chromium.org>
      Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
      Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
      Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
      Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
      Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
      Gerrit-Attention: Mason Freed <mas...@chromium.org>
      Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Attention: Thomas Nguyen <tun...@chromium.org>
      Gerrit-Comment-Date: Mon, 06 May 2024 18:28:26 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Andy Paicu <andy...@chromium.org>
      Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
      Comment-In-Reply-To: Aaron Leventhal <aleve...@chromium.org>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Andy Paicu (Gerrit)

      unread,
      May 6, 2024, 2:28:54 PM (12 days ago) May 6
      to Nektarios Paisios, Kevin Babbitt, (Julie)Jeongeun Kim, AyeAye, Aaron Leventhal, Mason Freed, Thomas Nguyen, Blink W3C Test Autoroller, Chromium LUCI CQ, chromium...@chromium.org, nektar...@chromium.org, josiah...@chromium.org, aleventhal...@chromium.org, dtseng...@chromium.org, blink-rev...@chromium.org, kyungjunle...@google.com, hirokisa...@chromium.org, yuzo+...@chromium.org, abigailbk...@google.com, francisjp...@google.com, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org
      Attention needed from Aaron Leventhal, Mason Freed and Thomas Nguyen

      Andy Paicu voted Commit-Queue+1

      Commit-Queue+1
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Aaron Leventhal
      • Mason Freed
      • Thomas Nguyen
      Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit