Bug 96727

Summary: EventSendingController::keyDown does not support non-array modifier arguments
Product: WebKit Reporter: Sudarsana Nagineni (babu) <naginenis>
Component: WebKit2Assignee: Sudarsana Nagineni (babu) <naginenis>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, eric, kenneth, mrobinson, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Sudarsana Nagineni (babu)
Reported 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.
Attachments
Patch (3.55 KB, patch)
2012-09-14 04:38 PDT, Sudarsana Nagineni (babu) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Sudarsana Nagineni (babu)
Comment 1 2012-09-14 04:38:34 PDT
Created attachment 164104 [details] Patch
WebKit Review Bot
Comment 2 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>
WebKit Review Bot
Comment 3 2012-09-14 06:19:16 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 4 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.
Sudarsana Nagineni (babu)
Comment 5 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
Alexey Proskuryakov
Comment 6 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.
Eric Seidel (no email)
Comment 7 2012-09-17 16:04:22 PDT
Seems like a reasonable feature to me. I certainly don't mind supporting this variant.
Sudarsana Nagineni (babu)
Comment 8 2012-10-01 01:35:31 PDT
Implemented this on Mac too, in the bug #97805
Note You need to log in before you can comment on or make changes to this bug.