Bug 96727 - EventSendingController::keyDown does not support non-array modifier arguments
Summary: EventSendingController::keyDown does not support non-array modifier arguments
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sudarsana Nagineni (babu)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-13 22:31 PDT by Sudarsana Nagineni (babu)
Modified: 2012-10-01 01:35 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.55 KB, patch)
2012-09-14 04:38 PDT, Sudarsana Nagineni (babu)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sudarsana Nagineni (babu) 2012-09-13 22:31:13 PDT
fast/forms/legend-access-key.html is failing since it uses "altKey" as the modifier argument. Non-array modifier arguments should be handled in EventSendingController::keyDown to solve the issue.
Comment 1 Sudarsana Nagineni (babu) 2012-09-14 04:38:34 PDT
Created attachment 164104 [details]
Patch
Comment 2 WebKit Review Bot 2012-09-14 06:19:14 PDT
Comment on attachment 164104 [details]
Patch

Clearing flags on attachment: 164104

Committed r128602: <http://trac.webkit.org/changeset/128602>
Comment 3 WebKit Review Bot 2012-09-14 06:19:16 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Alexey Proskuryakov 2012-09-14 09:49:27 PDT
> Non-array modifier arguments should be handled in EventSendingController::keyDown to solve the issue.

Are you saying that DumpRenderTree supports this, so WebKitTestRunner should, too? I don't see that in DumpRenderTree implementation.

I'd like to roll out this patch unless there is a convincing explanation.
Comment 5 Sudarsana Nagineni (babu) 2012-09-14 10:47:58 PDT
(In reply to comment #4)
> > Non-array modifier arguments should be handled in EventSendingController::keyDown to solve the issue.
> 
> Are you saying that DumpRenderTree supports this, so WebKitTestRunner should, too? I don't see that in DumpRenderTree implementation.

This support was added by different ports for DumpRenderTree in EventSender, so I thought it make sense to have it in WKTR too.

http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/efl/EventSender.cpp#L256
http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/gtk/EventSender.cpp#L297
http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/chromium/TestRunner/EventSender.cpp#L192
Comment 6 Alexey Proskuryakov 2012-09-17 16:01:08 PDT
CC'ing people who were adding this code to ports over the years.

This is just wrong. This was never meant to work with string arguments, and still doesn't in Mac DumpRenderTree. Was everyone just adding this code to accommodate for a typo in a single test?! How did that pass review multiple times?

Please at the very least implement this on Mac too, to regain interoperability of DumpRenderTree implementations.
Comment 7 Eric Seidel (no email) 2012-09-17 16:04:22 PDT
Seems like a reasonable feature to me.  I certainly don't mind supporting this variant.
Comment 8 Sudarsana Nagineni (babu) 2012-10-01 01:35:31 PDT
Implemented this on Mac too, in the bug #97805