(Copied from <http://crbug.com/13233>.) Chrome Version : 2.0.172.28 (Official Build ) URLs (if applicable) : Other browsers tested: Add OK or FAIL after other browsers where you have tested this issue: Safari 4: Fail Firefox 3.x: Fail IE 7: Fail IE 8: Fail What steps will reproduce the problem? 1. Take a flash movie with a text box 2. Render onto page with wmode parameter set to 'opaque' or 'transparent' 3. Set keyboard language to Greek 4. Type in box What is the expected result? Greek characters corresponding to keyboard layout What happens instead? Latin characters corresponding to US keyboard layout. Please provide any additional information below. Attach a screenshot if possible. I've made a demonstration page at http://baseonmars.co.uk/bugs/wmode along with short movie clips showing the results. It's possible to write text in another application (or even the address bar) then copy paste it into the textbox. The other browsers have this bug, but IE7, IE8 and Firefox Minefield demonstrate the bug by showing 2 characters for each keypress, generally one is a punctuation mark and the other shows as a rectangle. The firefox team have a long history with this bug, blame has been passed back and forth between the flash and mozilla team for at least 4 years (https://bugzilla.mozilla.org/show_bug.cgi?id=272847, https://bugzilla.mozilla.org/show_bug.cgi?id=347185). Hopefully some of their experience can help the chromium team.
Created attachment 49269 [details] A WebKit-side fix Greetings, The chromium-side fix for this issue is <http://codereview.chromium.org/267072/show>. Even though the new test included in this change fails without this chromium-side change, is it possible to review this WebKit-side change? Regards, Hironori Bono
Comment on attachment 49269 [details] A WebKit-side fix > Index: WebKit/chromium/src/WebViewImpl.cpp ... > @@ -507,8 +507,13 @@ bool WebViewImpl::keyEvent(const WebKeyb > PlatformKeyboardEventBuilder evt(event); > > if (handler->keyEvent(evt)) { > - if (WebInputEvent::RawKeyDown == event.type) > - m_suppressNextKeypressEvent = true; > + if (WebInputEvent::RawKeyDown == event.type) { > + // Suppress the next keypress event unless the focused node is a plug-in node. > + // (Flash needs these keypress events to handle non-US keyboards.) > + Node* node = frame->document()->focusedNode(); > + if (!node || (!node->hasTagName(HTMLNames::embedTag) && !node->hasTagName(HTMLNames::objectTag))) An <object> tag can be more than just a plug-in. It would probably be better check the renderer to see if it is a plug-in via node->renderer()->isEmbeddedObject(). Otherwise, I think this change is good. Nice work on the test!
Created attachment 50002 [details] A webKit-side fix 2 Thank you for your review and comments. (In reply to comment #2) > An <object> tag can be more than just a plug-in. It would > probably be better check the renderer to see if it is a plug-in > via node->renderer()->isEmbeddedObject(). Thank you for noticing this function. I wasn't aware that WebKit has such a convenient function. Since I have updated this change to use this function, is it possible to review the updated one? Regards, Hironori Bono
Comment on attachment 50002 [details] A webKit-side fix 2 ok.
Comment on attachment 50002 [details] A webKit-side fix 2 > Index: WebKit/chromium/src/WebViewImpl.cpp > =================================================================== > --- WebKit/chromium/src/WebViewImpl.cpp (revision 55511) > +++ WebKit/chromium/src/WebViewImpl.cpp (working copy) > @@ -507,8 +507,13 @@ bool WebViewImpl::keyEvent(const WebKeyb > PlatformKeyboardEventBuilder evt(event); > > if (handler->keyEvent(evt)) { > - if (WebInputEvent::RawKeyDown == event.type) > - m_suppressNextKeypressEvent = true; > + if (WebInputEvent::RawKeyDown == event.type) { > + // Suppress the next keypress event unless the focused node is a plug-in node. > + // (Flash needs these keypress events to handle non-US keyboards.) > + Node* node = frame->document()->focusedNode(); > + if (!node || !node->renderer()->isEmbeddedObject()) You might need to null check node->renderer() before de-referencing it. Most code does so.
Created attachment 50089 [details] A webkit-side fix 3 (In reply to comment #5) > You might need to null check node->renderer() before de-referencing it. > Most code does so. Thank you for your comment and sorry for my bonehead mistake. I have added the null check here. Is it possible to review the updated one? Regards, Hironori Bono
Comment on attachment 50002 [details] A webKit-side fix 2 Cleared Dimitri Glazkov's review+ from obsolete attachment 50002 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 50089 [details] A webkit-side fix 3 OK.
Comment on attachment 50089 [details] A webkit-side fix 3 Clearing flags on attachment: 50089 Committed r55620: <http://trac.webkit.org/changeset/55620>
All reviewed patches have been landed. Closing bug.