Bug 72359 - Access key should work on focusable element.
Summary: Access key should work on focusable element.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords: BrowserCompat, InRadar, WebExposed, WPTImpact
Depends on: 71854
Blocks: 234077
  Show dependency treegraph
 
Reported: 2011-11-15 00:30 PST by Vineet Chaudhary (vineetc)
Modified: 2022-01-02 21:54 PST (History)
20 users (show)

See Also:


Attachments
Test Case (524 bytes, text/html)
2011-11-15 00:30 PST, Vineet Chaudhary (vineetc)
no flags Details
proposed patch (4.42 KB, patch)
2011-11-15 01:55 PST, Vineet Chaudhary (vineetc)
no flags Details | Formatted Diff | Diff
Patch (9.44 KB, patch)
2021-12-27 23:11 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (10.40 KB, patch)
2021-12-28 00:13 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (13.08 KB, patch)
2021-12-29 05:48 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (12.60 KB, patch)
2021-12-29 09:21 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (13.25 KB, patch)
2021-12-30 02:52 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (12.30 KB, patch)
2021-12-30 03:21 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vineet Chaudhary (vineetc) 2011-11-15 00:30:56 PST
Created attachment 115113 [details]
Test Case

As per the specification http://dev.w3.org/html5/spec/Overview.html#the-accesskey-attribute all the elements can have the accesskey content attribute set.
In attached test case DIV element can support focus and should be focused with "accesssKey" attribute.

IE(7) : Is able to focus DIV.
Comment 1 Vineet Chaudhary (vineetc) 2011-11-15 01:55:10 PST
Created attachment 115127 [details]
proposed patch

Elements those can support focus should be focused on respective accessKey pressed.
Comment 2 Alexey Proskuryakov 2011-11-15 10:29:06 PST
Comment on attachment 115127 [details]
proposed patch

Implementing this in HTMLElement::accessKeyAction() means that elements are either focused, or get a simulated click (because HTMLAnchorElement and other subclasses override this method). This seems inconsistent.
Comment 3 Vineet Chaudhary (vineetc) 2011-11-15 12:17:33 PST
(In reply to comment #2)
> (From update of attachment 115127 [details])
> Implementing this in HTMLElement::accessKeyAction() means that elements are either focused, or get a simulated click (because HTMLAnchorElement and other subclasses override this method).

Thanks Alexey, for comments.

> This seems inconsistent.

I tried to make this generic behavior so that any element is focused, only if it supports focus. 
For other subclasses those overrides this method like
HTMLButtonElement/HTMLAnchorElement ==> Sends simulated click as these are click-able.
And For HTMLInputElement ==> Sets focus only as these are focusable.

Please correct me if I am getting this wrong.
Comment 4 Manuel Rego Casasnovas 2021-12-27 23:11:43 PST
Created attachment 448034 [details]
Patch
Comment 5 Manuel Rego Casasnovas 2021-12-27 23:15:11 PST
I was trying to understand why focus-visible-024.html was failing in wpt.fyi:
https://wpt.fyi/results/css/selectors/focus-visible-024.html?label=master&product=chrome%5Bexperimental%5D&product=edge%5Bexperimental%5D&product=firefox%5Bexperimental%5D&product=safari%5Bexperimental%5D&product=webkitgtk&aligned

And it looks like the reason is that a <DIV tabindex="0" accesskey="a"> is not focused via accesskey.

My patch to fix it is very similar to the original patch from Vineet, but also removing some implementations in the subclasses that won't be needed anymore.

This makes us match other browsers regarding accesskey.
Comment 6 Manuel Rego Casasnovas 2021-12-28 00:13:30 PST
Created attachment 448037 [details]
Patch
Comment 7 Darin Adler 2021-12-28 14:15:06 PST
Looks like the accessibility/mac/search-predicate-visited-links.html test is now failing. It failed on two different bots, so maybe it is indeed due to the changes here.
Comment 8 Manuel Rego Casasnovas 2021-12-29 05:45:06 PST
(In reply to Darin Adler from comment #7)
> Looks like the accessibility/mac/search-predicate-visited-links.html test is
> now failing. It failed on two different bots, so maybe it is indeed due to
> the changes here.

Yes that test gets affected by this change.

Thes test does this:
        accessibilityController.focusedElement.childAtIndex(0).childAtIndex(0).press();

Which ends up calling AccessibilityObject::press(), that uses accessKeyAction() (see https://github.com/WebKit/WebKit/blob/main/Source/WebCore/accessibility/AccessibilityObject.cpp#L1018).

I believe we can modify the test, because what's the focused element is not the main purpose of the test.
Comment 9 Manuel Rego Casasnovas 2021-12-29 05:48:15 PST
Created attachment 448068 [details]
Patch
Comment 10 Manuel Rego Casasnovas 2021-12-29 09:21:18 PST
Created attachment 448075 [details]
Patch
Comment 11 Manuel Rego Casasnovas 2021-12-30 02:52:28 PST
Created attachment 448089 [details]
Patch
Comment 12 Manuel Rego Casasnovas 2021-12-30 03:21:44 PST
Created attachment 448091 [details]
Patch
Comment 13 EWS 2022-01-02 21:53:49 PST
Committed r287529 (245664@main): <https://commits.webkit.org/245664@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448091 [details].
Comment 14 Radar WebKit Bug Importer 2022-01-02 21:54:27 PST
<rdar://problem/87058679>