Bug 75158 - Access keys do not work for frames that are not focused
Summary: Access keys do not work for frames that are not focused
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 19820
  Show dependency treegraph
 
Reported: 2011-12-22 19:17 PST by Cem Kocagil
Modified: 2012-01-05 16:34 PST (History)
6 users (show)

See Also:


Attachments
Test case (729 bytes, text/html)
2011-12-22 19:17 PST, Cem Kocagil
no flags Details
Patch (1.88 KB, patch)
2011-12-22 19:35 PST, Cem Kocagil
cem.kocagil+webkit: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (skips the layout test) (2.67 KB, patch)
2011-12-23 00:11 PST, Cem Kocagil
no flags Details | Formatted Diff | Diff
Patch (4.13 KB, patch)
2011-12-23 03:17 PST, Cem Kocagil
no flags Details | Formatted Diff | Diff
Patch (4.05 KB, patch)
2011-12-23 03:35 PST, Cem Kocagil
no flags Details | Formatted Diff | Diff
Patch (4.14 KB, patch)
2011-12-28 14:38 PST, Cem Kocagil
rniwa: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cem Kocagil 2011-12-22 19:17:14 PST
If the element targeted by the accesskey is not in a focused frame, it doesn't get selected (tested on Chromium and Safari). IE, Firefox and Opera focus on the item.

This behavior depends on port-specific code. This bug report is intended to represent the Chromium part of the problem and should block 19820.

See the attachment to reproduce the bug.
Comment 1 Cem Kocagil 2011-12-22 19:17:56 PST
Created attachment 120424 [details]
Test case
Comment 2 Cem Kocagil 2011-12-22 19:35:39 PST
Created attachment 120426 [details]
Patch
Comment 3 WebKit Review Bot 2011-12-22 21:17:37 PST
Comment on attachment 120426 [details]
Patch

Attachment 120426 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10939163

New failing tests:
fast/dom/access-key-iframe.html
Comment 4 Cem Kocagil 2011-12-23 00:11:07 PST
Created attachment 120439 [details]
Patch (skips the layout test)
Comment 5 Cem Kocagil 2011-12-23 03:17:04 PST
Created attachment 120449 [details]
Patch

Now with changelog; waiting to be reviewed.
Comment 6 WebKit Review Bot 2011-12-23 03:20:09 PST
Attachment 120449 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

LayoutTests/ChangeLog:10:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Cem Kocagil 2011-12-23 03:35:10 PST
Created attachment 120452 [details]
Patch
Comment 8 Cem Kocagil 2011-12-28 14:38:44 PST
Created attachment 120701 [details]
Patch
Comment 9 Ryosuke Niwa 2012-01-03 13:14:45 PST
Comment on attachment 120701 [details]
Patch

We can't test this?
Comment 10 Ryosuke Niwa 2012-01-03 13:19:56 PST
Comment on attachment 120701 [details]
Patch

I don't think it's right to only change the behavior of Chromium port given that all other major browsers exihit the same behavior and the code is shared in WebCore (handleAccessKey). We should modify handleAccessKey so that it'll go through all frames instead.
Comment 11 Alexey Proskuryakov 2012-01-03 13:38:54 PST
This looks security sensitive.

What guarantees that this doesn't introduce XSS? A frame could dispatch a keyboard event to another frame this way, or it could fool a user into pressing the access key combo, triggering an action in a different origin frame.
Comment 12 Cem Kocagil 2012-01-03 16:54:03 PST
(In reply to comment #11)
> This looks security sensitive.
> 
> What guarantees that this doesn't introduce XSS? A frame could dispatch a keyboard event to another frame this way, or it could fool a user into pressing the access key combo, triggering an action in a different origin frame.

It doesn't look like dispatched events can trigger WebViewImpl::charEvent and my simple tests verified this (I'm not quite sure though). It would be strange if that was the case since it's a method of the webview itself, not the Page or a Frame.

The calls to EventHandler::handleAccessKey does not send events to other frames, it only finds the element corresponding to that accesskey and invokes accessKeyAction on that element.
Comment 13 Alexey Proskuryakov 2012-01-05 16:34:35 PST
Thanks.

I still feel very uneasy (security-wise) about access keys working across multiple documents.