When a select popup is opened in Chromium, it steals activation from the browser, causing spurious blur/focus events to be sent on the page. The WebKit Chromium API should support making the select popup not activable.
Created attachment 50605 [details] Make the select popup non-activated
Attachment 50605 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/chromium/PopupMenuChromium.h:119: One space before end of line comments [whitespace/comments] [5] WebKit/chromium/src/WebViewImpl.cpp:844: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 2 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 50606 [details] Fixed style issues
Created attachment 50739 [details] Keeping the old WebViewClient method so not break the Chromium build.
Comment on attachment 50739 [details] Keeping the old WebViewClient method so not break the Chromium build. Extra space: 315 PopupContainer::~PopupContainer() Would appear we desparately need a convenience method for at least part of this: static_cast<ChromeClientChromium*>( 390 m_frameView->frame()->page()->chrome()->client())
Created attachment 51159 [details] Applied changes suggested by Eric
Attachment 51159 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/chromium/PopupMenuChromium.h:42: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 51159 [details] Applied changes suggested by Eric > Index: WebCore/page/chromium/ChromeClientChromium.h ... > + // Notifies the client a popup was closed. > + virtual void popupClosed(WebCore::PopupContainer* popupContainer) = 0; nit: ^^^ no need for WebCore:: prefix when already within the WebCore namespace. > Index: WebCore/platform/chromium/PopupMenuChromium.h ... > + // Convenience method that returns the ChromeClient of the frame view this > + // popup is associated with. > + ChromeClientChromium* chromeClientChromium(); nit: // Returns the ChromeClientChromium of the page this popup is associated with. > Index: WebKit/chromium/src/WebViewImpl.cpp ... > +bool WebViewImpl::selectPopupHandleKeyEvent(const WebKeyboardEvent& event) > +{ > + if (!m_selectPopup) > + return false; nit: ^^^ indentation It turns out that the rule for indentation within a namespace changed, and the guidance is to revise the indentation of files as we modify them. So, in this case, PopupContainer.h should have its style fixed. Otherwise, R=me
Created attachment 51297 [details] Fixing more style issues
Attachment 51297 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/chromium/PopupMenuChromium.h:63: One space before end of line comments [whitespace/comments] [5] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 51298 [details] One more style issue
Comment on attachment 51298 [details] One more style issue Clearing flags on attachment: 51298 Committed r56376: <http://trac.webkit.org/changeset/56376>
All reviewed patches have been landed. Closing bug.
This patch was reverted in http://trac.webkit.org/changeset/56416 due to consistent crashes in two tests on chromium linux ( fast/forms/select-empty-list.html and fast/forms/slider-delete-while-dragging-thumb.html ).
Created attachment 51469 [details] Fix a crasher on LayoutTests There was a crasher that could happen when a select popup is opened and a page navigates. The cause is that the PopupMenuContainer still exists but the PopupMenuClient has been deleted, and then when hidden the popup would try to access the PopupMenuClient, causing the crasher. We now clear the reference to the PopupMenuClient when the popup is deleted and we don't access it.
Comment on attachment 51469 [details] Fix a crasher on LayoutTests Clearing flags on attachment: 51469 Committed r56449: <http://trac.webkit.org/changeset/56449>
*** Bug 37423 has been marked as a duplicate of this bug. ***
This fix may have caused Bug 47769