Attachment 51298 Details for Bug 36062 – One more style issue

[patch] One more style issue

select.patch (text/plain), 27.68 KB, created by Jay Campan on 2010-03-22 10:00:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jay Campan

Created: 2010-03-22 10:00:53 PDT

Size: 27.68 KB

patch

obsolete

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 56014)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,26 @@
>+2010-03-15  Jay Campan  <jcampan@google.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Making Chromium select popups not steal activation from the browser.
>+        Select popups are now like autocomplete popups, shown in non-activated
>+        windows.
>+        https://bugs.webkit.org/show_bug.cgi?id=36062
>+
>+        * page/chromium/ChromeClientChromium.h:
>+        * platform/chromium/PopupMenuChromium.cpp:
>+        (WebCore::):
>+        (WebCore::PopupContainer::create):
>+        (WebCore::PopupContainer::PopupContainer):
>+        (WebCore::PopupContainer::~PopupContainer):
>+        (WebCore::PopupContainer::showPopup):
>+        (WebCore::PopupContainer::showExternal):
>+        (WebCore::PopupContainer::hidePopup):
>+        (WebCore::PopupMenu::show):
>+        * platform/chromium/PopupMenuChromium.h:
>+        (WebCore::PopupContainer::):
>+        (WebCore::PopupContainer::popupType):
>+
> 2010-03-15  Andy Estes  <aestes@apple.com>
> 
>         Reviewed by John Sullivan.
>Index: WebCore/page/chromium/ChromeClientChromium.h
>===================================================================
>--- WebCore/page/chromium/ChromeClientChromium.h	(revision 56012)
>+++ WebCore/page/chromium/ChromeClientChromium.h	(working copy)
>@@ -48,7 +48,10 @@ public:
>     // If handleExternal is true, then drawing and input handling for the
>     // popup will be handled by the external embedder.
>     virtual void popupOpened(PopupContainer* popupContainer, const IntRect& bounds,
>-                             bool focusOnShow, bool handleExternal) = 0;
>+                             bool handleExternal) = 0;
>+                             
>+    // Notifies the client a popup was closed.
>+    virtual void popupClosed(PopupContainer* popupContainer) = 0;
> 
>     // Notifies embedder that the state of an accessibility object has changed.
>     virtual void didChangeAccessibilityObjectState(AccessibilityObject*) = 0;
>Index: WebCore/platform/chromium/PopupMenuChromium.cpp
>===================================================================
>--- WebCore/platform/chromium/PopupMenuChromium.cpp	(revision 56012)
>+++ WebCore/platform/chromium/PopupMenuChromium.cpp	(working copy)
>@@ -74,7 +74,6 @@ static const TimeStamp kTypeAheadTimeout
> // The settings used for the drop down menu.
> // This is the delegate used if none is provided.
> static const PopupContainerSettings dropDownSettings = {
>-    true,   // focusOnShow
>     true,   // setTextOnIndexChange
>     true,   // acceptOnAbandon
>     false,  // loopSelectionNavigation
>@@ -297,14 +296,17 @@ static PlatformWheelEvent constructRelat
> 
> // static
> PassRefPtr<PopupContainer> PopupContainer::create(PopupMenuClient* client,
>+                                                  PopupType popupType,
>                                                   const PopupContainerSettings& settings)
> {
>-    return adoptRef(new PopupContainer(client, settings));
>+    return adoptRef(new PopupContainer(client, popupType, settings));
> }
> 
> PopupContainer::PopupContainer(PopupMenuClient* client,
>+                               PopupType popupType,
>                                const PopupContainerSettings& settings)
>     : m_listBox(PopupListBox::create(client, settings))
>+    , m_popupType(popupType)  
>     , m_settings(settings)
> {
>     setScrollbarModes(ScrollbarAlwaysOff, ScrollbarAlwaysOff);
>@@ -325,8 +327,8 @@ void PopupContainer::showPopup(FrameView
>     // WidgetClient about it.  It should assign us a client.
>     layout();
> 
>-    ChromeClientChromium* chromeClient = static_cast<ChromeClientChromium*>(
>-        view->frame()->page()->chrome()->client());
>+    m_frameView = view;
>+    ChromeClientChromium* chromeClient = chromeClientChromium();
>     if (chromeClient) {
>         // If the popup would extend past the bottom of the screen, open upwards
>         // instead.
>@@ -335,7 +337,7 @@ void PopupContainer::showPopup(FrameView
>         if (widgetRect.bottom() > static_cast<int>(screen.bottom()))
>             widgetRect.move(0, -(widgetRect.height() + selectHeight));
> 
>-        chromeClient->popupOpened(this, widgetRect, m_settings.focusOnShow, false);
>+        chromeClient->popupOpened(this, widgetRect, false);
>     }
> 
>     if (!m_listBox->parent())
>@@ -368,9 +370,8 @@ void PopupContainer::showExternal(const 
>     IntRect popupRect(location, rect.size());
> 
>     // Get the ChromeClient and pass it the popup menu's listbox data.
>-    ChromeClientChromium* client = static_cast<ChromeClientChromium*>(
>-         v->frame()->page()->chrome()->client());
>-    client->popupOpened(this, popupRect, true, true);
>+    m_frameView = v;
>+    chromeClientChromium()->popupOpened(this, popupRect, true);
> 
>     // The popup sends its "closed" notification through its parent. Set the
>     // parent, even though external popups have no real on-screen widget but a
>@@ -382,6 +383,7 @@ void PopupContainer::showExternal(const 
> void PopupContainer::hidePopup()
> {
>     listBox()->hidePopup();
>+    chromeClientChromium()->popupClosed(this);
> }
> 
> void PopupContainer::layout()
>@@ -470,6 +472,11 @@ bool PopupContainer::isInterestedInEvent
>     return m_listBox->isInterestedInEventForKey(keyCode);
> }
> 
>+ChromeClientChromium* PopupContainer::chromeClientChromium()
>+{
>+    return static_cast<ChromeClientChromium*>(m_frameView->frame()->page()->chrome()->client());
>+}
>+
> void PopupContainer::show(const IntRect& r, FrameView* v, int index)
> {
>     // The rect is the size of the select box. It's usually larger than we need.
>@@ -1231,7 +1238,7 @@ PopupMenu::~PopupMenu()
> void PopupMenu::show(const IntRect& r, FrameView* v, int index)
> {
>     if (!p.popup)
>-        p.popup = PopupContainer::create(client(), dropDownSettings);
>+        p.popup = PopupContainer::create(client(), PopupContainer::Select, dropDownSettings);
> #if OS(DARWIN)
>     p.popup->showExternal(r, v, index);
> #else
>Index: WebCore/platform/chromium/PopupMenuChromium.h
>===================================================================
>--- WebCore/platform/chromium/PopupMenuChromium.h	(revision 56012)
>+++ WebCore/platform/chromium/PopupMenuChromium.h	(working copy)
>@@ -39,149 +39,159 @@
> 
> namespace WebCore {
> 
>-    class FrameView;
>-    class PopupListBox;
>-
>-    // A container for the data for each menu item (e.g. represented by <option>
>-    // or <optgroup> in a <select> widget) and is used by PopupListBox.
>-    struct PopupItem {
>-        enum Type {
>-            TypeOption,
>-            TypeGroup,
>-            TypeSeparator
>-        };
>-
>-        PopupItem(const String& label, Type type)
>-            : label(label)
>-            , type(type)
>-            , yOffset(0)
>-        {
>-        }
>-        String label;
>-        Type type;
>-        int yOffset;  // y offset of this item, relative to the top of the popup.
>-        bool enabled;
>+class ChromeClientChromium;
>+class FrameView;
>+class PopupListBox;
>+
>+// A container for the data for each menu item (e.g. represented by <option>
>+// or <optgroup> in a <select> widget) and is used by PopupListBox.
>+struct PopupItem {
>+    enum Type {
>+        TypeOption,
>+        TypeGroup,
>+        TypeSeparator
>     };
> 
>-    // FIXME: Our FramelessScrollView classes should probably implement HostWindow!
>+    PopupItem(const String& label, Type type)
>+        : label(label)
>+        , type(type)
>+        , yOffset(0)
>+    {
>+    }
>+    String label;
>+    Type type;
>+    int yOffset; // y offset of this item, relative to the top of the popup.
>+    bool enabled;
>+};
>+
>+// FIXME: Our FramelessScrollView classes should probably implement HostWindow!
>+
>+// The PopupContainer class holds a PopupListBox (see cpp file).  Its sole purpose is to be
>+// able to draw a border around its child.  All its paint/event handling is
>+// just forwarded to the child listBox (with the appropriate transforms).
>+// NOTE: this class is exposed so it can be instantiated direcly for the
>+// autofill popup.  We cannot use the Popup class directly in that case as the
>+// autofill popup should not be focused when shown and we want to forward the
>+// key events to it (through handleKeyEvent).
>+
>+struct PopupContainerSettings {
>+    // Whether the PopupMenuClient should be told to change its text when a
>+    // new item is selected by using the arrow keys.
>+    bool setTextOnIndexChange;
>+
>+    // Whether the selection should be accepted when the popup menu is
>+    // closed (through ESC being pressed or the focus going away).
>+    // Note that when TAB is pressed, the selection is always accepted
>+    // regardless of this setting.
>+    bool acceptOnAbandon;
>+
>+    // Whether we should move the selection to the first/last item when
>+    // the user presses down/up arrow keys and the last/first item is
>+    // selected.
>+    bool loopSelectionNavigation;
>+
>+    // Whether we should restrict the width of the PopupListBox or not.
>+    // Autocomplete popups are restricted, combo-boxes (select tags) aren't.
>+    bool restrictWidthOfListBox;
>+
>+    // A hint on the display directionality of the item text in popup menu.
>+    //
>+    // We could either display the items in the drop-down using its DOM element's
>+    // directionality, or we could display the items in the drop-down using heuristics:
>+    // such as in its first strong directionality character's direction.
>+    // Please refer to the discussion (especially comment #7 and #10) in
>+    // https://bugs.webkit.org/show_bug.cgi?id=27889 for details.
>+    enum DirectionalityHint {
>+        // Use the DOM element's directionality to display the item text in popup menu.
>+        DOMElementDirection,
>+        // Use the item text's first strong-directional character's directionality
>+        // to display the item text in popup menu.
>+        FirstStrongDirectionalCharacterDirection,
>+    };
>+    DirectionalityHint itemTextDirectionalityHint;
>+};
> 
>-    // The PopupContainer class holds a PopupListBox (see cpp file).  Its sole purpose is to be
>-    // able to draw a border around its child.  All its paint/event handling is
>-    // just forwarded to the child listBox (with the appropriate transforms).
>-    // NOTE: this class is exposed so it can be instantiated direcly for the
>-    // autofill popup.  We cannot use the Popup class directly in that case as the
>-    // autofill popup should not be focused when shown and we want to forward the
>-    // key events to it (through handleKeyEvent).
>-
>-    struct PopupContainerSettings {
>-        // Whether the popup should get the focus when displayed.
>-        bool focusOnShow;
>-
>-        // Whether the PopupMenuClient should be told to change its text when a
>-        // new item is selected by using the arrow keys.
>-        bool setTextOnIndexChange;
>-
>-        // Whether the selection should be accepted when the popup menu is
>-        // closed (through ESC being pressed or the focus going away).
>-        // Note that when TAB is pressed, the selection is always accepted
>-        // regardless of this setting.
>-        bool acceptOnAbandon;
>-
>-        // Whether we should move the selection to the first/last item when
>-        // the user presses down/up arrow keys and the last/first item is
>-        // selected.
>-        bool loopSelectionNavigation;
>-
>-        // Whether we should restrict the width of the PopupListBox or not.
>-        // Autocomplete popups are restricted, combo-boxes (select tags) aren't.
>-        bool restrictWidthOfListBox;
>-
>-        // A hint on the display directionality of the item text in popup menu.
>-        //
>-        // We could either display the items in the drop-down using its DOM element's
>-        // directionality, or we could display the items in the drop-down using heuristics:
>-        // such as in its first strong directionality character's direction.
>-        // Please refer to the discussion (especially comment #7 and #10) in
>-        // https://bugs.webkit.org/show_bug.cgi?id=27889 for details.
>-        enum DirectionalityHint {
>-            // Use the DOM element's directionality to display the item text in popup menu.
>-            DOMElementDirection,
>-            // Use the item text's first strong-directional character's directionality
>-            // to display the item text in popup menu.
>-            FirstStrongDirectionalCharacterDirection,
>-        };
>-        DirectionalityHint itemTextDirectionalityHint;
>+class PopupContainer : public FramelessScrollView {
>+public:
>+    enum PopupType {
>+        Select, // HTML select popup.
>+        Suggestion, // Autocomplete/autofill popup.
>     };
> 
>-    class PopupContainer : public FramelessScrollView {
>-    public:
>-        static PassRefPtr<PopupContainer> create(PopupMenuClient*,
>-                                                 const PopupContainerSettings&);
>+    static PassRefPtr<PopupContainer> create(PopupMenuClient*, PopupType,
>+                                             const PopupContainerSettings&);
> 
>-        // Whether a key event should be sent to this popup.
>-        virtual bool isInterestedInEventForKey(int keyCode);
>+    // Whether a key event should be sent to this popup.
>+    virtual bool isInterestedInEventForKey(int keyCode);
> 
>-        // FramelessScrollView
>-        virtual void paint(GraphicsContext*, const IntRect&);
>-        virtual void hide();
>-        virtual bool handleMouseDownEvent(const PlatformMouseEvent&);
>-        virtual bool handleMouseMoveEvent(const PlatformMouseEvent&);
>-        virtual bool handleMouseReleaseEvent(const PlatformMouseEvent&);
>-        virtual bool handleWheelEvent(const PlatformWheelEvent&);
>-        virtual bool handleKeyEvent(const PlatformKeyboardEvent&);
>+    // FramelessScrollView
>+    virtual void paint(GraphicsContext*, const IntRect&);
>+    virtual void hide();
>+    virtual bool handleMouseDownEvent(const PlatformMouseEvent&);
>+    virtual bool handleMouseMoveEvent(const PlatformMouseEvent&);
>+    virtual bool handleMouseReleaseEvent(const PlatformMouseEvent&);
>+    virtual bool handleWheelEvent(const PlatformWheelEvent&);
>+    virtual bool handleKeyEvent(const PlatformKeyboardEvent&);
> 
>-        // PopupContainer methods
>+    // PopupContainer methods
> 
>-        // Show the popup
>-        void showPopup(FrameView*);
>+    // Show the popup
>+    void showPopup(FrameView*);
> 
>-        // Used on Mac Chromium for HTML select popup menus.
>-        void showExternal(const IntRect&, FrameView*, int index);
>+    // Used on Mac Chromium for HTML select popup menus.
>+    void showExternal(const IntRect&, FrameView*, int index);
> 
>-        // Show the popup in the specified rect for the specified frame.
>-        // Note: this code was somehow arbitrarily factored-out of the Popup class
>-        // so WebViewImpl can create a PopupContainer. This method is used for
>-        // displaying auto complete popup menus on Mac Chromium, and for all
>-        // popups on other platforms.
>-        void show(const IntRect&, FrameView*, int index);
>+    // Show the popup in the specified rect for the specified frame.
>+    // Note: this code was somehow arbitrarily factored-out of the Popup class
>+    // so WebViewImpl can create a PopupContainer. This method is used for
>+    // displaying auto complete popup menus on Mac Chromium, and for all
>+    // popups on other platforms.
>+    void show(const IntRect&, FrameView*, int index);
> 
>-        // Hide the popup.
>-        void hidePopup();
>+    // Hide the popup.
>+    void hidePopup();
> 
>-        // Compute size of widget and children.
>-        void layout();
>+    // Compute size of widget and children.
>+    void layout();
> 
>-        PopupListBox* listBox() const { return m_listBox.get(); }
>+    PopupListBox* listBox() const { return m_listBox.get(); }
> 
>-        // Gets the index of the item that the user is currently moused-over or
>-        // has selected with the keyboard up/down arrows.
>-        int selectedIndex() const;
>+    // Gets the index of the item that the user is currently moused-over or
>+    // has selected with the keyboard up/down arrows.
>+    int selectedIndex() const;
> 
>-        // Refresh the popup values from the PopupMenuClient.
>-        void refresh();
>+    // Refresh the popup values from the PopupMenuClient.
>+    void refresh();
> 
>-        // The menu per-item data.
>-        const WTF::Vector<PopupItem*>& popupData() const;
>+    // The menu per-item data.
>+    const WTF::Vector<PopupItem*>& popupData() const;
> 
>-        // The height of a row in the menu.
>-        int menuItemHeight() const;
>+    // The height of a row in the menu.
>+    int menuItemHeight() const;
> 
>-        // The size of the font being used.
>-        int menuItemFontSize() const;
>+    // The size of the font being used.
>+    int menuItemFontSize() const;
> 
>-    private:
>-        friend class WTF::RefCounted<PopupContainer>;
>+    PopupType popupType() const { return m_popupType; }
> 
>-        PopupContainer(PopupMenuClient*, const PopupContainerSettings&);
>-        ~PopupContainer();
>+private:
>+    friend class WTF::RefCounted<PopupContainer>;
> 
>-        // Paint the border.
>-        void paintBorder(GraphicsContext*, const IntRect&);
>+    PopupContainer(PopupMenuClient*, PopupType popupType, const PopupContainerSettings&);
>+    ~PopupContainer();
> 
>-        RefPtr<PopupListBox> m_listBox;
>+    // Paint the border.
>+    void paintBorder(GraphicsContext*, const IntRect&);
> 
>-        PopupContainerSettings m_settings;
>-    };
>+    // Returns the ChromeClient of the page this popup is associated with.
>+    ChromeClientChromium* chromeClientChromium();
>+
>+    RefPtr<PopupListBox> m_listBox;
>+    RefPtr<FrameView> m_frameView;
>+
>+    PopupContainerSettings m_settings;
>+    PopupType m_popupType;
>+};
> 
> } // namespace WebCore
> 
>Index: WebKit/chromium/ChangeLog
>===================================================================
>--- WebKit/chromium/ChangeLog	(revision 56014)
>+++ WebKit/chromium/ChangeLog	(working copy)
>@@ -1,3 +1,31 @@
>+2010-03-15  Jay Campan  <jcampan@google.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Making Chromium select popups not steal activation from the browser.
>+        Select popups are now like autocomplete popups, shown in non-activated
>+        windows.
>+        https://bugs.webkit.org/show_bug.cgi?id=36062
>+
>+        * public/WebViewClient.h:
>+        (WebKit::WebViewClient::createPopupMenu):
>+        * src/ChromeClientImpl.cpp:
>+        (WebKit::ChromeClientImpl::popupOpened):
>+        (WebKit::ChromeClientImpl::popupClosed):
>+        * src/ChromeClientImpl.h:
>+        * src/WebViewImpl.cpp:
>+        (WebKit::):
>+        (WebKit::WebViewImpl::mouseDown):
>+        (WebKit::WebViewImpl::keyEvent):
>+        (WebKit::WebViewImpl::selectPopupHandleKeyEvent):
>+        (WebKit::WebViewImpl::hideSelectPopup):
>+        (WebKit::WebViewImpl::popupOpened):
>+        (WebKit::WebViewImpl::popupClosed):
>+        (WebKit::WebViewImpl::setFocus):
>+        (WebKit::WebViewImpl::applyAutoFillSuggestions):
>+        (WebKit::WebViewImpl::applyAutocompleteSuggestions):
>+        * src/WebViewImpl.h:
>+
> 2010-03-15  Marcus Bulach  <bulach@chromium.org>
> 
>         Reviewed by Jeremy Orlow.
>Index: WebKit/chromium/public/WebViewClient.h
>===================================================================
>--- WebKit/chromium/public/WebViewClient.h	(revision 56012)
>+++ WebKit/chromium/public/WebViewClient.h	(working copy)
>@@ -73,8 +73,11 @@ public:
> 
>     // Create a new WebPopupMenu.  In the second form, the client is
>     // responsible for rendering the contents of the popup menu.
>-    virtual WebWidget* createPopupMenu(bool activatable) { return 0; }
>+    virtual WebWidget* createPopupMenu() { return 0; }
>     virtual WebWidget* createPopupMenu(const WebPopupMenuInfo&) { return 0; }
>+    // Deprecated method.
>+    virtual WebWidget* createPopupMenu(bool activatable) { return 0; }
>+
> 
>     // Create a session storage namespace object associated with this WebView.
>     virtual WebStorageNamespace* createSessionStorageNamespace() { return 0; }
>Index: WebKit/chromium/src/ChromeClientImpl.cpp
>===================================================================
>--- WebKit/chromium/src/ChromeClientImpl.cpp	(revision 56012)
>+++ WebKit/chromium/src/ChromeClientImpl.cpp	(working copy)
>@@ -594,7 +594,6 @@ void ChromeClientImpl::iconForFiles(cons
> 
> void ChromeClientImpl::popupOpened(PopupContainer* popupContainer,
>                                    const IntRect& bounds,
>-                                   bool activatable,
>                                    bool handleExternally)
> {
>     if (!m_webView->client())
>@@ -605,12 +604,22 @@ void ChromeClientImpl::popupOpened(Popup
>         WebPopupMenuInfo popupInfo;
>         getPopupMenuInfo(popupContainer, &popupInfo);
>         webwidget = m_webView->client()->createPopupMenu(popupInfo);
>-    } else
>-        webwidget = m_webView->client()->createPopupMenu(activatable);
>-
>+    } else {
>+        webwidget = m_webView->client()->createPopupMenu();
>+        if (!webwidget) {
>+            // Try the deprecated method.
>+            webwidget = m_webView->client()->createPopupMenu(false);
>+        }    
>+    }
>+    m_webView->popupOpened(popupContainer);
>     static_cast<WebPopupMenuImpl*>(webwidget)->Init(popupContainer, bounds);
> }
> 
>+void ChromeClientImpl::popupClosed(WebCore::PopupContainer* popupContainer)
>+{
>+    m_webView->popupClosed(popupContainer);
>+}
>+
> void ChromeClientImpl::setCursor(const WebCursorInfo& cursor)
> {
>     if (m_ignoreNextSetCursor) {
>Index: WebKit/chromium/src/ChromeClientImpl.h
>===================================================================
>--- WebKit/chromium/src/ChromeClientImpl.h	(revision 56012)
>+++ WebKit/chromium/src/ChromeClientImpl.h	(working copy)
>@@ -136,8 +136,8 @@ public:
>     // ChromeClientChromium methods:
>     virtual void popupOpened(WebCore::PopupContainer* popupContainer,
>                              const WebCore::IntRect& bounds,
>-                             bool activatable,
>                              bool handleExternally);
>+    virtual void popupClosed(WebCore::PopupContainer* popupContainer);
>     virtual void didChangeAccessibilityObjectState(WebCore::AccessibilityObject*);
> 
>     // ChromeClientImpl:
>Index: WebKit/chromium/src/WebViewImpl.cpp
>===================================================================
>--- WebKit/chromium/src/WebViewImpl.cpp	(revision 56012)
>+++ WebKit/chromium/src/WebViewImpl.cpp	(working copy)
>@@ -149,11 +149,7 @@ COMPILE_ASSERT_MATCHING_ENUM(DragOperati
> COMPILE_ASSERT_MATCHING_ENUM(DragOperationDelete);
> COMPILE_ASSERT_MATCHING_ENUM(DragOperationEvery);
> 
>-// Note that focusOnShow is false so that the suggestions popup is shown not
>-// activated.  We need the page to still have focus so the user can keep typing
>-// while the popup is showing.
> static const PopupContainerSettings suggestionsPopupSettings = {
>-    false,  // focusOnShow
>     false,  // setTextOnIndexChange
>     false,  // acceptOnAbandon
>     true,   // loopSelectionNavigation
>@@ -327,6 +323,10 @@ void WebViewImpl::mouseDown(const WebMou
>     if (!mainFrameImpl() || !mainFrameImpl()->frameView())
>         return;
> 
>+    // If there is a select popup opened, close it as the user is clicking on
>+    // the page (outside of the popup).
>+    hideSelectPopup();
>+
>     m_lastMouseDownPoint = WebPoint(event.x, event.y);
>     m_haveMouseCapture = true;
> 
>@@ -474,6 +474,10 @@ bool WebViewImpl::keyEvent(const WebKeyb
>     // event.
>     m_suppressNextKeypressEvent = false;
> 
>+    // Give any select popup a chance at consuming the key event.
>+    if (selectPopupHandleKeyEvent(event))
>+        return true;
>+
>     // Give Autocomplete a chance to consume the key events it is interested in.
>     if (autocompleteHandleKeyEvent(event))
>         return true;
>@@ -523,6 +527,14 @@ bool WebViewImpl::keyEvent(const WebKeyb
>     return keyEventDefault(event);
> }
> 
>+bool WebViewImpl::selectPopupHandleKeyEvent(const WebKeyboardEvent& event)
>+{
>+    if (!m_selectPopup)
>+        return false;
>+    
>+    return m_selectPopup->handleKeyEvent(PlatformKeyboardEventBuilder(event));
>+}
>+
> bool WebViewImpl::autocompleteHandleKeyEvent(const WebKeyboardEvent& event)
> {
>     if (!m_suggestionsPopupShowing
>@@ -793,6 +805,12 @@ bool WebViewImpl::scrollViewWithKeyboard
>     return propagateScroll(scrollDirection, scrollGranularity);
> }
> 
>+void WebViewImpl::hideSelectPopup()
>+{
>+    if (m_selectPopup.get())
>+        m_selectPopup->hidePopup();
>+}
>+
> bool WebViewImpl::propagateScroll(ScrollDirection scrollDirection,
>                                   ScrollGranularity scrollGranularity)
> {
>@@ -812,6 +830,22 @@ bool WebViewImpl::propagateScroll(Scroll
>     return scrollHandled;
> }
> 
>+void  WebViewImpl::popupOpened(WebCore::PopupContainer* popupContainer)
>+{
>+    if (popupContainer->popupType() == WebCore::PopupContainer::Select) {
>+        ASSERT(!m_selectPopup);
>+        m_selectPopup = popupContainer;
>+    }
>+}
>+
>+void  WebViewImpl::popupClosed(WebCore::PopupContainer* popupContainer)
>+{
>+    if (popupContainer->popupType() == WebCore::PopupContainer::Select) {
>+        ASSERT(m_selectPopup.get());
>+        m_selectPopup = 0;
>+    }
>+}
>+
> Frame* WebViewImpl::focusedWebCoreFrame()
> {
>     return m_page.get() ? m_page->focusController()->focusedOrMainFrame() : 0;
>@@ -1043,6 +1077,7 @@ void WebViewImpl::setFocus(bool enable)
>         m_imeAcceptEvents = true;
>     } else {
>         hideSuggestionsPopup();
>+        hideSelectPopup();
> 
>         // Clear focus on the currently focused frame if any.
>         if (!m_page.get())
>@@ -1668,6 +1703,7 @@ void WebViewImpl::applyAutoFillSuggestio
> 
>     if (!m_autoFillPopup.get()) {
>         m_autoFillPopup = PopupContainer::create(m_suggestionsPopupClient,
>+                                                 PopupContainer::Suggestion,
>                                                  suggestionsPopupSettings);
>     }
> 
>@@ -1723,6 +1759,7 @@ void WebViewImpl::applyAutocompleteSugge
> 
>     if (!m_autocompletePopup.get()) {
>         m_autocompletePopup = PopupContainer::create(m_suggestionsPopupClient,
>+                                                     PopupContainer::Suggestion,
>                                                      suggestionsPopupSettings);
>     }
> 
>Index: WebKit/chromium/src/WebViewImpl.h
>===================================================================
>--- WebKit/chromium/src/WebViewImpl.h	(revision 56012)
>+++ WebKit/chromium/src/WebViewImpl.h	(working copy)
>@@ -290,6 +290,10 @@ public:
>     // was scrolled.
>     bool propagateScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity);
> 
>+    // Notification that a popup was opened/closed.
>+    void popupOpened(WebCore::PopupContainer* popupContainer);
>+    void popupClosed(WebCore::PopupContainer* popupContainer);
>+
>     // HACK: currentInputEvent() is for ChromeClientImpl::show(), until we can
>     // fix WebKit to pass enough information up into ChromeClient::show() so we
>     // can decide if the window.open event was caused by a middle-mouse click
>@@ -308,6 +312,9 @@ private:
>     // Returns true if the event was actually processed.
>     bool keyEventDefault(const WebKeyboardEvent&);
> 
>+    // Returns true if the select popup has consumed the event.
>+    bool selectPopupHandleKeyEvent(const WebKeyboardEvent&);
>+
>     // Returns true if the autocomple has consumed the event.
>     bool autocompleteHandleKeyEvent(const WebKeyboardEvent&);
> 
>@@ -319,6 +326,9 @@ private:
>     // Returns true if the view was scrolled.
>     bool scrollViewWithKeyboard(int keyCode, int modifiers);
> 
>+    // Hides the select popup if one is opened.
>+    void hideSelectPopup();
>+
>     // Converts |pos| from window coordinates to contents coordinates and gets
>     // the HitTestResult for it.
>     WebCore::HitTestResult hitTestResultForWindowPos(const WebCore::IntPoint&);
>@@ -426,6 +436,9 @@ private:
>     // A pointer to the current suggestions popup.  We do not own this pointer.
>     WebCore::PopupContainer* m_suggestionsPopup;
> 
>+    // The popup associated with a select element.
>+    RefPtr<WebCore::PopupContainer> m_selectPopup;
>+
>     // The AutoFill suggestions popup.
>     RefPtr<WebCore::PopupContainer> m_autoFillPopup;
>

Actions: View | Formatted Diff | Diff

Attachments on bug 36062: 50605 | 50606 | 50739 | 51159 | 51297 | 51298 | 51469