Bug 83014 – [Chromium] PlatformKeyboardEvent::m_windowsVirtualKeyCode should be zero for char events

• Status: RESOLVED
• Resolution: WONTFIX
• Component: UI Events

[Cem Kocagil]

Chromium creates PlatformKeyboardEvents with char type that return non-zero for windowsVirtualKeyCode(). It shouldn't, as documented in PlatformKeyboardEvent.h ("Zero for Char events").

[Cem Kocagil]

Created attachment 135304 [details]
Patch

[WebKit Review Bot]

Comment on attachment 135304 [details]
Patch

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

New failing tests:
fullscreen/full-screen-placeholder.html
fullscreen/full-screen-zIndex-after.html
fullscreen/full-screen-twice.html
fullscreen/full-screen-css.html
fullscreen/full-screen-keyboard-disabled.html

[WebKit Review Bot]

Created attachment 135309 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Cem Kocagil]

Created attachment 135340 [details]
Patch

[WebKit Review Bot]

Attachment 135340 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/page/EventHandler.cpp:2729:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Cem Kocagil]

Created attachment 135342 [details]
Patch

[Cem Kocagil]

Created attachment 135940 [details]
Patch

[WebKit Review Bot]

Comment on attachment 135940 [details]
Patch

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

New failing tests:
fullscreen/full-screen-placeholder.html
fullscreen/full-screen-zIndex-after.html
fullscreen/full-screen-twice.html
fullscreen/full-screen-css.html
fullscreen/full-screen-keyboard-disabled.html

[WebKit Review Bot]

Created attachment 135959 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Cem Kocagil]

Created attachment 136963 [details]
Rebased

[Ryosuke Niwa]

Comment on attachment 136963 [details]
Rebased

View in context: https://bugs.webkit.org/attachment.cgi?id=136963&action=review

> Source/WebKit/chromium/ChangeLog:7
> +

Please explain why you're making this change. r- due to the lack of description.
Also, can't this be tested by a chromium unit test?

[Cem Kocagil]

(In reply to comment #11)
> Please explain why you're making this change. r- due to the lack of description.
> Also, can't this be tested by a chromium unit test?

I explained the reason behind this change in the first comment, is it required to explain in the ChangeLog too?

You're probably right, this should be able to be tested (if there currently are unit tests around that part of the code).

[Stephen Chenney]

https://code.google.com/p/chromium/issues/detail?id=231311