Bug 83014 - [Chromium] PlatformKeyboardEvent::m_windowsVirtualKeyCode should be zero for char events
Summary: [Chromium] PlatformKeyboardEvent::m_windowsVirtualKeyCode should be zero for ...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 83330
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-03 03:37 PDT by Cem Kocagil
Modified: 2013-04-15 06:57 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.49 KB, patch)
2012-04-03 04:01 PDT, Cem Kocagil
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-02 (6.65 MB, application/zip)
2012-04-03 04:42 PDT, WebKit Review Bot
no flags Details
Patch (3.23 KB, patch)
2012-04-03 09:05 PDT, Cem Kocagil
no flags Details | Formatted Diff | Diff
Patch (2.79 KB, patch)
2012-04-03 09:11 PDT, Cem Kocagil
cem.kocagil+webkit: review-
Details | Formatted Diff | Diff
Patch (1.50 KB, patch)
2012-04-05 17:08 PDT, Cem Kocagil
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (6.46 MB, application/zip)
2012-04-05 18:40 PDT, WebKit Review Bot
no flags Details
Rebased (1.44 KB, patch)
2012-04-12 13:49 PDT, 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 2012-04-03 03:37:00 PDT
Chromium creates PlatformKeyboardEvents with char type that return non-zero for windowsVirtualKeyCode(). It shouldn't, as documented in PlatformKeyboardEvent.h ("Zero for Char events").
Comment 1 Cem Kocagil 2012-04-03 04:01:21 PDT
Created attachment 135304 [details]
Patch
Comment 2 WebKit Review Bot 2012-04-03 04:42:34 PDT
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
Comment 3 WebKit Review Bot 2012-04-03 04:42:40 PDT
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
Comment 4 Cem Kocagil 2012-04-03 09:05:41 PDT
Created attachment 135340 [details]
Patch
Comment 5 WebKit Review Bot 2012-04-03 09:08:48 PDT
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.
Comment 6 Cem Kocagil 2012-04-03 09:11:49 PDT
Created attachment 135342 [details]
Patch
Comment 7 Cem Kocagil 2012-04-05 17:08:10 PDT
Created attachment 135940 [details]
Patch
Comment 8 WebKit Review Bot 2012-04-05 18:40:00 PDT
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
Comment 9 WebKit Review Bot 2012-04-05 18:40:06 PDT
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
Comment 10 Cem Kocagil 2012-04-12 13:49:29 PDT
Created attachment 136963 [details]
Rebased
Comment 11 Ryosuke Niwa 2012-04-22 22:14:26 PDT
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?
Comment 12 Cem Kocagil 2012-04-26 06:54:37 PDT
(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).