Changeset 87920 in webkit


Ignore:
Timestamp:
Jun 2, 2011 9:45:01 AM (13 years ago)
Author:
commit-queue@webkit.org
Message:

2011-06-02 Naoki Takano <​takano.naoki@gmail.com>

Reviewed by Dimitri Glazkov.

[Chromium] Click event is not fired for a menulist <select>
​https://bugs.webkit.org/show_bug.cgi?id=60563

Tests: SelectItemEventFire, SelectItemKeyEvent, SelectItemRemoveSelectOnChange and SelectItemRemoveSelectOnChange in webkit_unit_tests.

  • platform/chromium/PopupMenuChromium.cpp: (WebCore::PopupContainer::showPopup): Set m_focusedNode from m_frameView. (WebCore::PopupListBox::handleMouseReleaseEvent): Call dispatchMouseEvent to forward the event only if select popup. (WebCore::PopupListBox::acceptIndex): Change to return accepted or not.

2011-06-02 Naoki Takano <​takano.naoki@gmail.com>

Reviewed by Dimitri Glazkov.

[Chromium] Click event is not fired for a menulist <select>
​https://bugs.webkit.org/show_bug.cgi?id=60563

  • tests/PopupMenuTest.cpp: (WebKit::TestPopupMenuClient::TestPopupMenuClient): Initialize m_node. (WebKit::TestPopupMenuClient::valueChanged): To fire 'change' event, forward the event like RenderMenuList. (WebKit::TestPopupMenuClient::itemIsEnabled): Change to return true or false according to disabled item or not. (WebKit::TestPopupMenuClient::setDisabledIndex): Set disabled index to simulate disabled item. (WebKit::TestPopupMenuClient::setFocusedNode): Set focused node to dispatch the event. (WebKit::SelectPopupMenuTest::SelectPopupMenuTest): Add baseURL. (WebKit::SelectPopupMenuTest::TearDown): Add UnregisterAllMockedURLs() call. (WebKit::SelectPopupMenuTest::registerMockedURLLoad): To simulate html load, call RegisterMockedURL(). (WebKit::SelectPopupMenuTest::serveRequests): Call ServeAsynchronousMockedRequests(). (WebKit::SelectPopupMenuTest::loadFrame): Simulate load frame with url string. (WebKit::TEST_F): Implement SelectItemEventFire, SelectItemKeyEvent, SelectItemRemoveSelectOnChange and SelectItemRemoveSelectOnChange.
  • tests/data/select_event.html: Added for SelectItemEventFire and SelectItemKeyEvent.
  • tests/data/select_event_remove_on_change.html: Added SelectItemRemoveSelectOnChange.
  • tests/data/select_event_remove_on_click.html: Added SelectItemRemoveSelectOnChange.
Location:
trunk/Source
Files:
4 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r87914 r87920  
     12011-06-02  Naoki Takano  <takano.naoki@gmail.com>
     2
     3        Reviewed by Dimitri Glazkov.
     4
     5        [Chromium] Click event is not fired for a menulist <select>
     6        https://bugs.webkit.org/show_bug.cgi?id=60563
     7
     8        Tests: SelectItemEventFire, SelectItemKeyEvent, SelectItemRemoveSelectOnChange and SelectItemRemoveSelectOnChange in webkit_unit_tests.
     9
     10        * platform/chromium/PopupMenuChromium.cpp:
     11        (WebCore::PopupContainer::showPopup): Set m_focusedNode from m_frameView.
     12        (WebCore::PopupListBox::handleMouseReleaseEvent): Call dispatchMouseEvent to forward the event only if select popup.
     13        (WebCore::PopupListBox::acceptIndex): Change to return accepted or not.
     14
    1152011-06-02  Dimitri Glazkov  <dglazkov@chromium.org>
    216

  • trunk/Source/WebCore/platform/chromium/PopupMenuChromium.cpp

    r87137 r87920  
    189189
    190190    // Accepts the selected index as the value to be displayed in the <select> widget on
    191     // the web page, and closes the popup.
    192     void acceptIndex(int index);
     191    // the web page, and closes the popup. Returns true if index is accepted.
     192    bool acceptIndex(int index);
    193193
    194194    // Clears the selection (so no row appears selected).
    … …  
    278278    // If width exeeds screen width, we have to clip it.
    279279    int m_maxWindowWidth;
     280
     281    // To forward last mouse release event.
     282    RefPtr<Node> m_focusedNode;
    280283};
    281284
    … …  
    408411{
    409412    m_frameView = view;
     413    listBox()->m_focusedNode = m_frameView->frame()->document()->focusedNode();
    410414
    411415    ChromeClientChromium* chromeClient = chromeClientChromium();
    … …  
    673677        return true;
    674678
    675     acceptIndex(pointToRowIndex(event.pos()));
     679    // Need to check before calling acceptIndex(), because m_popupClient might be removed in acceptIndex() calling because of event handler.
     680    bool isSelectPopup = m_popupClient->menuStyle().menuType() == PopupMenuStyle::SelectPopup;
     681    if (acceptIndex(pointToRowIndex(event.pos())) && m_focusedNode && isSelectPopup) {
     682        m_focusedNode->dispatchMouseEvent(event, eventNames().mouseupEvent);
     683        m_focusedNode->dispatchMouseEvent(event, eventNames().clickEvent);
     684
     685        // Clear m_focusedNode here, because we cannot clear in hidePopup() which is called before dispatchMouseEvent() is called.
     686        m_focusedNode = 0;
     687    }
     688
    676689    return true;
    677690}
    … …  
    10691082}
    10701083
    1071 void PopupListBox::acceptIndex(int index)
     1084bool PopupListBox::acceptIndex(int index)
    10721085{
    10731086    // Clear m_acceptedIndexOnAbandon once user accepts the selected index.
    … …  
    10761089
    10771090    if (index >= numItems())
    1078         return;
     1091        return false;
    10791092
    10801093    if (index < 0) {
    … …  
    10831096            hidePopup();
    10841097        }
    1085         return;
     1098        return false;
    10861099    }
    10871100
    … …  
    10941107        // Tell the <select> PopupMenuClient what index was selected.
    10951108        m_popupClient->valueChanged(index);
    1096     }
     1109
     1110        return true;
     1111    }
     1112
     1113    return false;
    10971114}
    10981115

  • trunk/Source/WebKit/chromium/ChangeLog

    r87868 r87920  
     12011-06-02  Naoki Takano  <takano.naoki@gmail.com>
     2
     3        Reviewed by Dimitri Glazkov.
     4
     5        [Chromium] Click event is not fired for a menulist <select>
     6        https://bugs.webkit.org/show_bug.cgi?id=60563
     7
     8        * tests/PopupMenuTest.cpp:
     9        (WebKit::TestPopupMenuClient::TestPopupMenuClient): Initialize m_node.
     10        (WebKit::TestPopupMenuClient::valueChanged): To fire 'change' event, forward the event like RenderMenuList.
     11        (WebKit::TestPopupMenuClient::itemIsEnabled): Change to return true or false according to disabled item or not.
     12        (WebKit::TestPopupMenuClient::setDisabledIndex): Set disabled index to simulate disabled item.
     13        (WebKit::TestPopupMenuClient::setFocusedNode): Set focused node to dispatch the event.
     14        (WebKit::SelectPopupMenuTest::SelectPopupMenuTest): Add baseURL.
     15        (WebKit::SelectPopupMenuTest::TearDown): Add UnregisterAllMockedURLs() call.
     16        (WebKit::SelectPopupMenuTest::registerMockedURLLoad): To simulate html load, call RegisterMockedURL().
     17        (WebKit::SelectPopupMenuTest::serveRequests): Call ServeAsynchronousMockedRequests().
     18        (WebKit::SelectPopupMenuTest::loadFrame): Simulate load frame with url string.
     19        (WebKit::TEST_F): Implement SelectItemEventFire, SelectItemKeyEvent, SelectItemRemoveSelectOnChange and SelectItemRemoveSelectOnChange.
     20        * tests/data/select_event.html: Added for SelectItemEventFire and SelectItemKeyEvent.
     21        * tests/data/select_event_remove_on_change.html: Added SelectItemRemoveSelectOnChange.
     22        * tests/data/select_event_remove_on_click.html: Added SelectItemRemoveSelectOnChange.
     23
    1242011-06-01  Adrienne Walker  <enne@google.com>
    225

  • trunk/Source/WebKit/chromium/tests/PopupMenuTest.cpp

    r85093 r87920  
    3232
    3333#include <gtest/gtest.h>
     34#include <webkit/support/webkit_support.h>
    3435
    3536#include "Color.h"
     37#include "Element.h"
     38#include "FrameView.h"
    3639#include "KeyboardCodes.h"
    3740#include "PopupMenu.h"
    3841#include "PopupMenuClient.h"
    3942#include "PopupMenuChromium.h"
     43#include "SelectElement.h"
     44#include "WebDocument.h"
     45#include "WebElement.h"
     46#include "WebFrame.h"
    4047#include "WebFrameClient.h"
    4148#include "WebFrameImpl.h"
    … …  
    4350#include "WebPopupMenuImpl.h"
    4451#include "WebScreenInfo.h"
     52#include "WebSettings.h"
     53#include "WebString.h"
     54#include "WebURL.h"
     55#include "WebURLRequest.h"
     56#include "WebURLResponse.h"
     57#include "WebView.h"
    4558#include "WebViewClient.h"
    4659#include "WebViewImpl.h"
     60#include "v8.h"
    4761
    4862using namespace WebCore;
    … …  
    5468public:
    5569    // Item at index 0 is selected by default.
    56     TestPopupMenuClient() : m_selectIndex(0) { }
     70    TestPopupMenuClient() : m_selectIndex(0), m_node(0) { }
    5771    virtual ~TestPopupMenuClient() {}
    5872    virtual void valueChanged(unsigned listIndex, bool fireEvents = true)
    5973    {
    6074        m_selectIndex = listIndex;
     75        if (m_node) {
     76            SelectElement* select = toSelectElement(static_cast<Element*>(m_node));
     77            select->setSelectedIndexByUser(select->listToOptionIndex(listIndex), true, fireEvents);
     78        }
    6179    }
    6280    virtual void selectionChanged(unsigned, bool) {}
    … …  
    7391    virtual String itemToolTip(unsigned listIndex) const { return itemText(listIndex); }
    7492    virtual String itemAccessibilityText(unsigned listIndex) const { return itemText(listIndex); }
    75     virtual bool itemIsEnabled(unsigned listIndex) const { return true; }
     93    virtual bool itemIsEnabled(unsigned listIndex) const { return m_disabledIndexSet.find(listIndex) == m_disabledIndexSet.end(); }
    7694    virtual PopupMenuStyle itemStyle(unsigned listIndex) const
    7795    {
    … …  
    93111    virtual bool valueShouldChangeOnHotTrack() const { return false; }
    94112    virtual void setTextFromItem(unsigned listIndex) { }
    95    
     113
    96114    virtual FontSelector* fontSelector() const { return 0; }
    97115    virtual HostWindow* hostWindow() const { return 0; }
    98    
     116
    99117    virtual PassRefPtr<Scrollbar> createScrollbar(ScrollableArea*, ScrollbarOrientation, ScrollbarControlSize) { return 0; }
     118
     119    void setDisabledIndex(unsigned index) { m_disabledIndexSet.insert(index); }
     120    void setFocusedNode(Node* node) { m_node = node; }
    100121
    101122private:
    102123    unsigned m_selectIndex;
     124    std::set<unsigned> m_disabledIndexSet;
     125    Node* m_node;
    103126};
    104127
    … …  
    151174public:
    152175    SelectPopupMenuTest()
     176        : baseURL("http://www.test.com/")
    153177    {
    154178    }
    … …  
    166190        m_popupMenu = 0;
    167191        m_webView->close();
     192        webkit_support::UnregisterAllMockedURLs();
    168193    }
    169194
    … …  
    219244                                      1, false, false, false, false, 0);
    220245        m_webView->selectPopup()->handleMouseReleaseEvent(mouseEvent);
     246    }
     247
     248    void registerMockedURLLoad(const std::string& fileName)
     249    {
     250        WebURLResponse response;
     251        response.initialize();
     252        response.setMIMEType("text/html");
     253
     254        std::string filePath = webkit_support::GetWebKitRootDir().utf8();
     255        filePath += "/Source/WebKit/chromium/tests/data/popup/";
     256        filePath += fileName;
     257
     258        webkit_support::RegisterMockedURL(WebURL(GURL(baseURL + fileName)), response, WebString::fromUTF8(filePath));
     259    }
     260
     261    void serveRequests()
     262    {
     263        webkit_support::ServeAsynchronousMockedRequests();
     264    }
     265
     266    void loadFrame(WebFrame* frame, const std::string& fileName)
     267    {
     268        WebURLRequest urlRequest;
     269        urlRequest.initialize();
     270        urlRequest.setURL(WebURL(GURL(baseURL + fileName)));
     271        frame->loadRequest(urlRequest);
    221272    }
    222273
    … …  
    227278    TestPopupMenuClient m_popupMenuClient;
    228279    RefPtr<PopupMenu> m_popupMenu;
     280    std::string baseURL;
    229281};
    230282
    … …  
    351403}
    352404
     405TEST_F(SelectPopupMenuTest, SelectItemEventFire)
     406{
     407    registerMockedURLLoad("select_event.html");
     408    m_webView->settings()->setJavaScriptEnabled(true);
     409    loadFrame(m_webView->mainFrame(), "select_event.html");
     410    serveRequests();
     411
     412    m_popupMenuClient.setFocusedNode(static_cast<WebFrameImpl*>(m_webView->mainFrame())->frameView()->frame()->document()->focusedNode());
     413
     414    showPopup();
     415
     416    int menuHeight = m_webView->selectPopup()->menuItemHeight();
     417    // menuHeight * 0.5 means the Y position on the item at index 0.
     418    IntPoint row1Point(2, menuHeight * 0.5);
     419    simulateLeftMouseDownEvent(row1Point);
     420    simulateLeftMouseUpEvent(row1Point);
     421
     422    WebElement element = m_webView->mainFrame()->document().getElementById("message");
     423
     424    // mousedown event is held by select node, and we don't simulate the event for the node.
     425    // So we can only see mouseup and click event.
     426    EXPECT_STREQ("upclick", std::string(element.innerText().utf8()).c_str());
     427
     428    // Disable the item at index 1.
     429    m_popupMenuClient.setDisabledIndex(1);
     430
     431    showPopup();
     432    // menuHeight * 1.5 means the Y position on the item at index 1.
     433    row1Point.setY(menuHeight * 1.5);
     434    simulateLeftMouseDownEvent(row1Point);
     435    simulateLeftMouseUpEvent(row1Point);
     436
     437    // The item at index 1 is disabled, so the text should not be changed.
     438    EXPECT_STREQ("upclick", std::string(element.innerText().utf8()).c_str());
     439
     440    showPopup();
     441    // menuHeight * 2.5 means the Y position on the item at index 2.
     442    row1Point.setY(menuHeight * 2.5);
     443    simulateLeftMouseDownEvent(row1Point);
     444    simulateLeftMouseUpEvent(row1Point);
     445
     446    // The item is changed to the item at index 2, from index 0, so change event is fired.
     447    EXPECT_STREQ("upclickchangeupclick", std::string(element.innerText().utf8()).c_str());
     448}
     449
     450TEST_F(SelectPopupMenuTest, SelectItemKeyEvent)
     451{
     452    registerMockedURLLoad("select_event.html");
     453    m_webView->settings()->setJavaScriptEnabled(true);
     454    loadFrame(m_webView->mainFrame(), "select_event.html");
     455    serveRequests();
     456
     457    m_popupMenuClient.setFocusedNode(static_cast<WebFrameImpl*>(m_webView->mainFrame())->frameView()->frame()->document()->focusedNode());
     458
     459    showPopup();
     460
     461    // Siumulate to choose the item at index 1 with keyboard.
     462    simulateKeyDownEvent(VKEY_DOWN);
     463    simulateKeyDownEvent(VKEY_DOWN);
     464    simulateKeyDownEvent(VKEY_RETURN);
     465
     466    WebElement element = m_webView->mainFrame()->document().getElementById("message");
     467    // We only can see change event but no other mouse related events.
     468    EXPECT_STREQ("change", std::string(element.innerText().utf8()).c_str());
     469}
     470
     471TEST_F(SelectPopupMenuTest, SelectItemRemoveSelectOnChange)
     472{
     473    // Make sure no crash, even if select node is removed on 'change' event handler.
     474    registerMockedURLLoad("select_event_remove_on_change.html");
     475    m_webView->settings()->setJavaScriptEnabled(true);
     476    loadFrame(m_webView->mainFrame(), "select_event_remove_on_change.html");
     477    serveRequests();
     478
     479    m_popupMenuClient.setFocusedNode(static_cast<WebFrameImpl*>(m_webView->mainFrame())->frameView()->frame()->document()->focusedNode());
     480
     481    showPopup();
     482
     483    int menuHeight = m_webView->selectPopup()->menuItemHeight();
     484    // menuHeight * 1.5 means the Y position on the item at index 1.
     485    IntPoint row1Point(2, menuHeight * 1.5);
     486    simulateLeftMouseDownEvent(row1Point);
     487    simulateLeftMouseUpEvent(row1Point);
     488
     489    WebElement element = m_webView->mainFrame()->document().getElementById("message");
     490    EXPECT_STREQ("change", std::string(element.innerText().utf8()).c_str());
     491}
     492
     493TEST_F(SelectPopupMenuTest, SelectItemRemoveSelectOnClick)
     494{
     495    // Make sure no crash, even if select node is removed on 'click' event handler.
     496    registerMockedURLLoad("select_event_remove_on_click.html");
     497    m_webView->settings()->setJavaScriptEnabled(true);
     498    loadFrame(m_webView->mainFrame(), "select_event_remove_on_click.html");
     499    serveRequests();
     500
     501    m_popupMenuClient.setFocusedNode(static_cast<WebFrameImpl*>(m_webView->mainFrame())->frameView()->frame()->document()->focusedNode());
     502
     503    showPopup();
     504
     505    int menuHeight = m_webView->selectPopup()->menuItemHeight();
     506    // menuHeight * 1.5 means the Y position on the item at index 1.
     507    IntPoint row1Point(2, menuHeight * 1.5);
     508    simulateLeftMouseDownEvent(row1Point);
     509    simulateLeftMouseUpEvent(row1Point);
     510
     511    WebElement element = m_webView->mainFrame()->document().getElementById("message");
     512    EXPECT_STREQ("click", std::string(element.innerText().utf8()).c_str());
     513}
     514
    353515} // namespace
Note: See TracChangeset for help on using the changeset viewer.