DevTools: accessibility fix for tab from prompt (issue 2538233004 by luoe@chromium.org)

0 views
Skip to first unread message

lu...@chromium.org

unread,
Dec 2, 2016, 4:41:16 PM12/2/16
to einb...@chromium.org, lush...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
Reviewers: einbinder, lushnikov
CL: https://codereview.chromium.org/2538233004/

Message:
Please take a look.

Description:
DevTools: accessibility fix for tab from prompt

TextPrompt used to always consume 'Tab' keys, preventing the use of tab to
navigate from a prompt to checkboxes. This CL makes the handler consume only if
autocomplete is handled.

BUG=668161

Affected files (+1, -4 lines):
M third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js


Index: third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js
diff --git a/third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js b/third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js
index a661c55b58ac966ff1cf71918602b4e3a2820148..195cc51734d4b2d9de97c8e2badf4535db0ae120 100644
--- a/third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js
+++ b/third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js
@@ -609,10 +609,7 @@ UI.TextPrompt = class extends Common.Object {
* @return {boolean}
*/
tabKeyPressed(event) {
- this.acceptAutoComplete();
-
- // Consume the key.
- return true;
+ return this.acceptAutoComplete();
}

/**


lush...@chromium.org

unread,
Dec 2, 2016, 8:23:39 PM12/2/16
to lu...@chromium.org, einb...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
does this change preserve the tab behavior in Styles sidebar pane?

https://codereview.chromium.org/2538233004/

lu...@chromium.org

unread,
Dec 2, 2016, 8:57:39 PM12/2/16
to einb...@chromium.org, lush...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
On 2016/12/03 01:23:39, lushnikov wrote:
> does this change preserve the tab behavior in Styles sidebar pane?

Yes, SSP.js contains logic that preserves its behavior where pressing tab will
always move to the next field. This logic is also covered by an existing test:
LayoutTests/inspector/elements/styles-3/styles-add-new-rule-tab.html

One effect of this CL is that pressing tab in a FilteredListWidget's prompt may
leave focus. I believe einbinder@ has a plan to have FilteredListWidget
intercept keyDown on its prompt, though.

https://codereview.chromium.org/2538233004/

dgo...@chromium.org

unread,
Dec 4, 2016, 1:59:56 PM12/4/16
to lu...@chromium.org, einb...@chromium.org, lush...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
> One effect of this CL is that pressing tab in a FilteredListWidget's prompt
may
> leave focus. I believe einbinder@ has a plan to have FilteredListWidget
> intercept keyDown on its prompt, though.

Where does the focus go then? I think this is a big regression.


https://codereview.chromium.org/2538233004/

lu...@chromium.org

unread,
Dec 5, 2016, 1:53:20 PM12/5/16
to einb...@chromium.org, lush...@chromium.org, chromium...@chromium.org, caseq...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, blink-...@chromium.org, pfel...@chromium.org, kozyatins...@chromium.org
I'm not sure what I was thinking, I don't intend to introduce this regression.
Focus will move into the browser URL bar and even the target page. Let me
revise this CL first.

https://codereview.chromium.org/2538233004/
Reply all
Reply to author
Forward
0 new messages