Bug 66679 – [chromium] DRT linux should pass nativeKeyCode to plugins

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Tony Chang]

From https://bugs.webkit.org/show_bug.cgi?id=65964#c4 :

> Do we know why we get an empty keycode with Chromium's DRT?  It seems to be working for GTK+.

The GTK port creates an honest to goodness GdkEvent keypress event
  http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/gtk/EventSender.cpp?rev=85516#L612

Chromium's DRT generates a WebKeyboardEvent, defined by the webkit api
  http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/chromium/EventSender.cpp?rev=85516#L566

and fills in the WebKeyboardEvent.windowsKeyCode field, but not the nativeKeyCode field
  http://trac.webkit.org/browser/trunk/WebKit/chromium/public/WebInputEvent.h?rev=50721#L153

The plugin is sent the nativeKeyCode value.  That works in the real browser case, but not with
the Chromium DRT.  Is there a table somewhere that maps webkit::VKEY_CODE to native key codes?

--

I don't think a table like this exists, but adding one for DRT would be fine.

[noel gordon]

Will require two-sided patch, chromium side is http://crbug/66679

[noel gordon]

Meant http://crbug.com/66679

[noel gordon]

copy/paste-fu-day, it's http://crbug.com/94211

[noel gordon]

Created attachment 105142 [details]
patch

[Tony Chang]

Comment on attachment 105142 [details]
patch

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

> Tools/DumpRenderTree/chromium/EventSender.cpp:580
> +    eventDown.nativeKeyCode = ui::NativeKeyCodeFromWindowsKeyCode(keyCode, needsShiftKeyModifier);

We don't want to call code in ui directly.  Instead, we want all methods to go through the webkit_support API.  You can add a function to webkit_support.h that does this conversion.  In general, this approach seems fine.

[noel gordon]

Created attachment 105300 [details]
Patch

Right, move the helper into the webkit_support namespace.

[noel gordon]

Chrome side part #1 done.
  http://src.chromium.org/viewvc/chrome?view=rev&revision=98599

[noel gordon]

Chrome side part #2 done.
  http://src.chromium.org/viewvc/chrome?view=rev&revision=98771

[noel gordon]

Wait for chrome rev 98771 to roll into webkit chromium DEPS.

[noel gordon]

DEPS rolled rev 98771 in http://trac.webkit.org/changeset/94086

[noel gordon]

Created attachment 105738 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 105738 [details]
Patch for landing

Clearing flags on attachment: 105738

Committed r94203: <http://trac.webkit.org/changeset/94203>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Fady Samuel]

(In reply to comment #13)
> All reviewed patches have been landed.  Closing bug.

DumpRenderTree seems to be broken now. 

CXX(target) out/Debug/obj.target/DumpRenderTree/third_party/WebKit/Tools/DumpRenderTree/chromium/WebViewHost.o
third_party/WebKit/Tools/DumpRenderTree/chromium/EventSender.cpp: In member function 'void EventSender::keyDown(const CppArgumentList&, CppVariant*)':
third_party/WebKit/Tools/DumpRenderTree/chromium/EventSender.cpp:580: error: 'NativeKeyCodeForWindowsKeyCode' is not a member of 'webkit_support'
make: *** [out/Debug/obj.target/DumpRenderTree/third_party/WebKit/Tools/DumpRenderTree/chromium/EventSender.o] Error 1

[noel gordon]

http://src.chromium.org/viewvc/chrome?view=rev&revision=104013

[noel gordon]

Created attachment 113778 [details]
Patch

[noel gordon]

webkit DEPS rolled past chromium r104013 4 weeks ago
  http://trac.webkit.org/changeset/96899/trunk/Source/WebKit/chromium/DEPS

And you'd think we would have heard about Linux DumpRenderTree compilation
failures 4 weeks ago.

Checked Linux DumpRenderTree is compiling on queues.webkit.org as we speak
  http://queues.webkit.org/results/10338386

I don't see a problem, possible false alarm?

[Tony Chang]

(In reply to comment #14)
> (In reply to comment #13)
> > All reviewed patches have been landed.  Closing bug.
> 
> DumpRenderTree seems to be broken now. 
> 
> CXX(target) out/Debug/obj.target/DumpRenderTree/third_party/WebKit/Tools/DumpRenderTree/chromium/WebViewHost.o
> third_party/WebKit/Tools/DumpRenderTree/chromium/EventSender.cpp: In member function 'void EventSender::keyDown(const CppArgumentList&, CppVariant*)':
> third_party/WebKit/Tools/DumpRenderTree/chromium/EventSender.cpp:580: error: 'NativeKeyCodeForWindowsKeyCode' is not a member of 'webkit_support'
> make: *** [out/Debug/obj.target/DumpRenderTree/third_party/WebKit/Tools/DumpRenderTree/chromium/EventSender.o] Error 1

Fady, which builder do you see this on?

[noel gordon]

Fady worked it out in http://trac.webkit.org/changeset/101464