[webkit-reviews] review granted: [Bug 97161] Mac Chromium: Ignore system numpad modifier : [Attachment 167594] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Sailesh Agrawal
<sail at chromium.org>'s request for review:
Bug 97161: Mac Chromium: Ignore system numpad modifier
https://bugs.webkit.org/show_bug.cgi?id=97161

Attachment 167594: Patch
https://bugs.webkit.org/attachment.cgi?id=167594&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167594&action=review


It sounds like we can't actually test this behavior in a layout test because
the event created by eventSender doesn't go through the port specific WebKit
code.  I would just revert the changes to DRT and the layout test.  The
unittest you added seems sufficient to test the change to Chromium's WebKit
code.  Trying to make DRT smarter just makes the layout test more confusing.

> Source/WebKit/chromium/WebKit.gypi:136
> +		       'tests/WebInputEventFactoryTestMac.mm',

Nit: I think you can put this in the main webkit_unittest_files list and it'll
get removed on Win/Linux because of the .mm extension.

> Source/WebKit/chromium/tests/WebInputEventFactoryTestMac.mm:43
> +using WebKit::WebInputEvent;
> +using WebKit::WebKeyboardEvent;
> +using WebKit::WebMouseEvent;
> +using WebKit::WebInputEventFactory;

Nit: Sort.

> Source/WebKit/chromium/tests/WebInputEventFactoryTestMac.mm:62
> +}  // namespace

Nit: WebKit style is only one space before // comments.