Changeset 207544 in webkit

Timestamp:

Oct 19, 2016 10:46:42 AM (8 years ago)

Author:

Chris Dumez

Message:

MouseEvent's coordinates should be 0 for simulated clicks
​https://bugs.webkit.org/show_bug.cgi?id=163648

Reviewed by Darin Adler.

Source/WebCore:

MouseEvent's coordinates should be 0 / 0 for simulated clicks triggered
by JavaScript (i.e. via element.click()). This behavior matches Chrome
and Firefox.

WebKit was computing actual coordinates for the element which was
expensive, especially because computing screenX / screenY required
a synchronous IPC with the UI process.

Test: fast/events/element-click-no-coords.html

• dom/Element.cpp:

(WebCore::Element::dispatchSimulatedClick):

• dom/SimulatedClick.cpp:

(WebCore::simulateMouseEvent):
(WebCore::simulateClick):

• dom/SimulatedClick.h:
• html/HTMLElement.cpp:

(WebCore::HTMLElement::click):

LayoutTests:

Add layout test coverage. I verified that this test is passing in
Firefox and Chrome as well.

• fast/events/element-click-no-coords-expected.txt: Added.
• fast/events/element-click-no-coords.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/element-click-no-coords-expected.txt (added)
• LayoutTests/fast/events/element-click-no-coords.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Element.cpp (modified) (1 diff)
• Source/WebCore/dom/SimulatedClick.cpp (modified) (4 diffs)
• Source/WebCore/dom/SimulatedClick.h (modified) (1 diff)
• Source/WebCore/html/HTMLElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-10-19  Chris Dumez  <cdumez@apple.com>
+
+        MouseEvent's coordinates should be 0 for simulated clicks
+        https://bugs.webkit.org/show_bug.cgi?id=163648
+
+        Reviewed by Darin Adler.
+
+        Add layout test coverage. I verified that this test is passing in
+        Firefox and Chrome as well.
+
+        * fast/events/element-click-no-coords-expected.txt: Added.
+        * fast/events/element-click-no-coords.html: Added.
+
 2016-10-19  Nan Wang  <n_wang@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-10-19  Chris Dumez  <cdumez@apple.com>
+
+        MouseEvent's coordinates should be 0 for simulated clicks
+        https://bugs.webkit.org/show_bug.cgi?id=163648
+
+        Reviewed by Darin Adler.
+
+        MouseEvent's coordinates should be 0 / 0 for simulated clicks triggered
+        by JavaScript (i.e. via element.click()). This behavior matches Chrome
+        and Firefox.
+
+        WebKit was computing actual coordinates for the element which was
+        expensive, especially because computing  screenX / screenY required
+        a synchronous IPC with the UI process.
+
+        Test: fast/events/element-click-no-coords.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::dispatchSimulatedClick):
+        * dom/SimulatedClick.cpp:
+        (WebCore::simulateMouseEvent):
+        (WebCore::simulateClick):
+        * dom/SimulatedClick.h:
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::click):
+
 2016-10-19  Dave Hyatt  <hyatt@apple.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -336 +336 @@
 void Element::dispatchSimulatedClick(Event* underlyingEvent, SimulatedClickMouseEventOptions eventOptions, SimulatedClickVisualOptions visualOptions)
 {
-    simulateClick(*this, underlyingEvent, eventOptions, visualOptions, SimulatedClickCreationOptions::FromUserAgent);
+    simulateClick(*this, underlyingEvent, eventOptions, visualOptions, SimulatedClickSource::UserAgent);
 }
 

trunk/Source/WebCore/dom/SimulatedClick.cpp

@@ -39 +39 @@
 class SimulatedMouseEvent final : public MouseEvent {
 public:
-    static Ref<SimulatedMouseEvent> create(const AtomicString& eventType, DOMWindow* view, RefPtr<Event>&& underlyingEvent, Element& target)
+    static Ref<SimulatedMouseEvent> create(const AtomicString& eventType, DOMWindow* view, RefPtr<Event>&& underlyingEvent, Element& target, SimulatedClickSource source)
     {
-        return adoptRef(*new SimulatedMouseEvent(eventType, view, WTFMove(underlyingEvent), target));
+        return adoptRef(*new SimulatedMouseEvent(eventType, view, WTFMove(underlyingEvent), target, source));
     }
 
 private:
-    SimulatedMouseEvent(const AtomicString& eventType, DOMWindow* view, RefPtr<Event>&& underlyingEvent, Element& target)
+    SimulatedMouseEvent(const AtomicString& eventType, DOMWindow* view, RefPtr<Event>&& underlyingEvent, Element& target, SimulatedClickSource source)
         : MouseEvent(eventType, true, true, underlyingEvent ? underlyingEvent->timeStamp() : currentTime(), view, 0, 0, 0, 0, 0,
 #if ENABLE(POINTER_LOCK)
@@ -52 +52 @@
                      false, false, false, false, 0, 0, 0, 0, 0, true)
     {
+        if (source == SimulatedClickSource::Bindings)
+            setUntrusted();
+
         if (UIEventWithKeyState* keyStateEvent = findEventWithKeyState(underlyingEvent.get())) {
             m_ctrlKey = keyStateEvent->ctrlKey();
@@ -64 +67 @@
             m_screenLocation = mouseEvent.screenLocation();
             initCoordinates(mouseEvent.clientLocation());
-        } else {
+        } else if (source == SimulatedClickSource::UserAgent) {
+            // If there is no underlying event, we only populate the coordinates for events coming
+            // from the user agent (e.g. accessibility). For those coming from JavaScript (e.g.
+            // (element.click()), the coordinates will be 0, similarly to Firefox and Chrome.
+            // Note that the call to screenRect() causes a synchronous IPC with the UI process.
             m_screenLocation = target.screenRect().center();
             initCoordinates(LayoutPoint(target.clientRect().center()));
@@ -72 +79 @@
 };
 
-static void simulateMouseEvent(const AtomicString& eventType, Element& element, Event* underlyingEvent, SimulatedClickCreationOptions creationOptions)
+static void simulateMouseEvent(const AtomicString& eventType, Element& element, Event* underlyingEvent, SimulatedClickSource source)
 {
-    auto event = SimulatedMouseEvent::create(eventType, element.document().defaultView(), underlyingEvent, element);
-    if (creationOptions == SimulatedClickCreationOptions::FromBindings)
-        event.get().setUntrusted();
-    EventDispatcher::dispatchEvent(&element, event.get());
+    auto event = SimulatedMouseEvent::create(eventType, element.document().defaultView(), underlyingEvent, element, source);
+    EventDispatcher::dispatchEvent(&element, event);
 }
 
-void simulateClick(Element& element, Event* underlyingEvent, SimulatedClickMouseEventOptions mouseEventOptions, SimulatedClickVisualOptions visualOptions, SimulatedClickCreationOptions creationOptions)
+void simulateClick(Element& element, Event* underlyingEvent, SimulatedClickMouseEventOptions mouseEventOptions, SimulatedClickVisualOptions visualOptions, SimulatedClickSource creationOptions)
 {
     if (element.isDisabledFormControl())

trunk/Source/WebCore/dom/SimulatedClick.h

@@ -34 +34 @@
 class Event;
 
-enum class SimulatedClickCreationOptions {
-    FromBindings,
-    FromUserAgent
+enum class SimulatedClickSource {
+    Bindings,
+    UserAgent
 };
 
-void simulateClick(Element&, Event* underlyingEvent, SimulatedClickMouseEventOptions, SimulatedClickVisualOptions, SimulatedClickCreationOptions);
+void simulateClick(Element&, Event* underlyingEvent, SimulatedClickMouseEventOptions, SimulatedClickVisualOptions, SimulatedClickSource);
 
 } // namespace WebCore

trunk/Source/WebCore/html/HTMLElement.cpp

@@ -702 +702 @@
 void HTMLElement::click()
 {
-    simulateClick(*this, nullptr, SendNoEvents, DoNotShowPressedLook, SimulatedClickCreationOptions::FromBindings);
+    simulateClick(*this, nullptr, SendNoEvents, DoNotShowPressedLook, SimulatedClickSource::Bindings);
 }