Attachment 349229 Details for Bug 188561 – Fixes the bug

[patch] Fixes the bug

bug-188561-20180907191143.patch (text/plain), 21.08 KB, created by Ryosuke Niwa on 2018-09-07 19:11:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-09-07 19:11:44 PDT

Size: 21.08 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 235825)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,33 @@
>+2018-09-07  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        mouseenter and mouseleave events don't get dispatched even when there is a capturing event listener for a slot ancestor
>+        https://bugs.webkit.org/show_bug.cgi?id=188561
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch makes mouseenter and mouseleave events to work with shadow trees and slots therein, and makes them uncomposed
>+        as discussed in https://github.com/w3c/uievents/issues/208.
>+
>+        This patch also makes these events dispatched on DOM tree ancestors of the currently hovered element instead of
>+        render tree's hover ancestors to be consistent with the check in hierarchyHasCapturingEventListeners and other browsers.
>+        In particular, using hover ancestors is problematic when there is an element with display: contents such as slot elements,
>+        which do not have a render object.
>+
>+        Tests: fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary.html
>+               fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree.html
>+               fast/shadow-dom/mouseenter-mouseleave-on-slot-parent.html
>+
>+        * dom/MouseEvent.cpp:
>+        (WebCore::MouseEvent::create):
>+        * page/EventHandler.cpp:
>+        (WebCore::nearestCommonHoverAncestor): Deleted.
>+        (WebCore::hierarchyHasCapturingEventListeners): Use parentInComposedTree. Else we would miss capturing event listeners
>+        on inclusive ancestors of slots.
>+        (WebCore::EventHandler::updateMouseEventTargetNode): Use the composed tree's ancestor chain to fire mouseenter and
>+        mouseleave events. This is needed to dispatch mouseenter / mouseleave events on slot elements. Also removed comments
>+        which just state what is self-evident from the code beneath them.
>+
>+
> 2018-09-07  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [Win][Clang] exceptionShouldTerminateProgram of StructuredExceptionHandlerSuppressor.cpp should take DWORD
>Index: Source/WebCore/dom/MouseEvent.cpp
>===================================================================
>--- Source/WebCore/dom/MouseEvent.cpp	(revision 235806)
>+++ Source/WebCore/dom/MouseEvent.cpp	(working copy)
>@@ -49,8 +49,9 @@ Ref<MouseEvent> MouseEvent::create(const
>     bool isMouseEnterOrLeave = eventType == eventNames().mouseenterEvent || eventType == eventNames().mouseleaveEvent;
>     auto isCancelable = eventType != eventNames().mousemoveEvent && !isMouseEnterOrLeave ? IsCancelable::Yes : IsCancelable::No;
>     auto canBubble = !isMouseEnterOrLeave ? CanBubble::Yes : CanBubble::No;
>+    auto isComposed = !isMouseEnterOrLeave ? IsComposed::Yes : IsComposed::No;
> 
>-    return MouseEvent::create(eventType, canBubble, isCancelable, IsComposed::Yes, event.timestamp().approximateMonotonicTime(), WTFMove(view), detail,
>+    return MouseEvent::create(eventType, canBubble, isCancelable, isComposed, event.timestamp().approximateMonotonicTime(), WTFMove(view), detail,
>         event.globalPosition(), event.position(),
> #if ENABLE(POINTER_LOCK)
>         event.movementDelta(),
>Index: Source/WebCore/page/EventHandler.cpp
>===================================================================
>--- Source/WebCore/page/EventHandler.cpp	(revision 235806)
>+++ Source/WebCore/page/EventHandler.cpp	(working copy)
>@@ -33,6 +33,7 @@
> #include "CachedImage.h"
> #include "Chrome.h"
> #include "ChromeClient.h"
>+#include "ComposedTreeAncestorIterator.h"
> #include "CursorList.h"
> #include "DocumentMarkerController.h"
> #include "DragController.h"
>@@ -2444,24 +2445,9 @@ MouseEventWithHitTestResults EventHandle
>     return m_frame.document()->prepareMouseEvent(request, documentPointForWindowPoint(m_frame, mouseEvent.position()), mouseEvent);
> }
> 
>-static RenderElement* nearestCommonHoverAncestor(RenderElement* obj1, RenderElement* obj2)
>-{
>-    if (!obj1 || !obj2)
>-        return nullptr;
>-
>-    for (RenderElement* currObj1 = obj1; currObj1; currObj1 = currObj1->hoverAncestor()) {
>-        for (RenderElement* currObj2 = obj2; currObj2; currObj2 = currObj2->hoverAncestor()) {
>-            if (currObj1 == currObj2)
>-                return currObj1;
>-        }
>-    }
>-
>-    return nullptr;
>-}
>-
> static bool hierarchyHasCapturingEventListeners(Element* element, const AtomicString& eventName)
> {
>-    for (ContainerNode* curr = element; curr; curr = curr->parentOrShadowHostNode()) {
>+    for (ContainerNode* curr = element; curr; curr = curr->parentInComposedTree()) {
>         if (curr->hasCapturingEventListeners(eventName))
>             return true;
>     }
>@@ -2535,48 +2521,35 @@ void EventHandler::updateMouseEventTarge
>             bool hasCapturingMouseEnterListener = hierarchyHasCapturingEventListeners(m_elementUnderMouse.get(), eventNames().mouseenterEvent);
>             bool hasCapturingMouseLeaveListener = hierarchyHasCapturingEventListeners(m_lastElementUnderMouse.get(), eventNames().mouseleaveEvent);
> 
>-            RenderElement* oldHoverRenderer = m_lastElementUnderMouse ? m_lastElementUnderMouse->renderer() : nullptr;
>-            RenderElement* newHoverRenderer = m_elementUnderMouse ? m_elementUnderMouse->renderer() : nullptr;
>-            RenderElement* ancestor = nearestCommonHoverAncestor(oldHoverRenderer, newHoverRenderer);
>-
>             Vector<Ref<Element>, 32> leftElementsChain;
>-            if (oldHoverRenderer) {
>-                for (RenderElement* curr = oldHoverRenderer; curr && curr != ancestor; curr = curr->hoverAncestor()) {
>-                    if (Element* element = curr->element())
>-                        leftElementsChain.append(*element);
>-                }
>-            } else {
>-                // If the old hovered element is not null but it's renderer is, it was probably detached.
>-                // In this case, the old hovered element (and its ancestors) must be updated, to ensure it's normal style is re-applied.
>-                for (Element* element = m_lastElementUnderMouse.get(); element; element = element->parentElement())
>-                    leftElementsChain.append(*element);
>-            }
>-
>+            for (Element* element = m_lastElementUnderMouse.get(); element; element = element->parentElementInComposedTree())
>+                leftElementsChain.append(*element);
>             Vector<Ref<Element>, 32> enteredElementsChain;
>-            const Element* ancestorElement = ancestor ? ancestor->element() : nullptr;
>-            for (RenderElement* curr = newHoverRenderer; curr; curr = curr->hoverAncestor()) {
>-                if (Element *element = curr->element()) {
>-                    if (element == ancestorElement)
>+            for (Element* element = m_elementUnderMouse.get(); element; element = element->parentElementInComposedTree())
>+                enteredElementsChain.append(*element);
>+
>+            if (!leftElementsChain.isEmpty() && !enteredElementsChain.isEmpty() && leftElementsChain.last().ptr() == enteredElementsChain.last().ptr()) {
>+                size_t minHeight = std::min(leftElementsChain.size(), enteredElementsChain.size());
>+                size_t i;
>+                for (i = 0; i < minHeight; ++i) {
>+                    if (leftElementsChain[leftElementsChain.size() - i - 1].ptr() != enteredElementsChain[enteredElementsChain.size() - i - 1].ptr())
>                         break;
>-                    enteredElementsChain.append(*element);
>                 }
>+                leftElementsChain.shrink(leftElementsChain.size() - i);
>+                enteredElementsChain.shrink(enteredElementsChain.size() - i);
>             }
> 
>-            // Send mouseout event to the old node.
>             if (m_lastElementUnderMouse)
>                 m_lastElementUnderMouse->dispatchMouseEvent(platformMouseEvent, eventNames().mouseoutEvent, 0, m_elementUnderMouse.get());
> 
>-            // Send mouseleave to the node hierarchy no longer under the mouse.
>             for (auto& chain : leftElementsChain) {
>                 if (hasCapturingMouseLeaveListener || chain->hasEventListeners(eventNames().mouseleaveEvent))
>                     chain->dispatchMouseEvent(platformMouseEvent, eventNames().mouseleaveEvent, 0, m_elementUnderMouse.get());
>             }
> 
>-            // Send mouseover event to the new node.
>             if (m_elementUnderMouse)
>                 m_elementUnderMouse->dispatchMouseEvent(platformMouseEvent, eventNames().mouseoverEvent, 0, m_lastElementUnderMouse.get());
> 
>-            // Send mouseleave event to the nodes hierarchy under the mouse.
>             for (auto& chain : enteredElementsChain) {
>                 if (hasCapturingMouseEnterListener || chain->hasEventListeners(eventNames().mouseenterEvent))
>                     chain->dispatchMouseEvent(platformMouseEvent, eventNames().mouseenterEvent, 0, m_lastElementUnderMouse.get());
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 235806)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2018-09-07  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        mouseenter and mouseleave events don't get dispatched even when there is a capturing event listener for a slot ancestor
>+        https://bugs.webkit.org/show_bug.cgi?id=188561
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added tests for listening to mouseenter and mouseleave events using capturing and bubbling event listeners
>+        across shadow boundaries.
>+
>+        * fast/events/shadow-event-path-expected.txt: Rebaselined now that mouseenter and mouseleave events are dispatched
>+        on ancestors of slots of the summary element's UA shadow tree as expected.
>+        * fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary-expected.txt: Added.
>+        * fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary.html: Added.
>+        * fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree-expected.txt: Added.
>+        * fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree.html: Added.
>+        * fast/shadow-dom/mouseenter-mouseleave-on-slot-parent-expected.txt: Added.
>+        * fast/shadow-dom/mouseenter-mouseleave-on-slot-parent.html: Added.
>+
> 2018-09-07  Frederic Wang  <fwang@igalia.com>
> 
>         [CSSOM View] Handle the scrollingElement in Element::scroll(Left/Top/Width/Height/To)
>Index: LayoutTests/fast/events/shadow-event-path-expected.txt
>===================================================================
>--- LayoutTests/fast/events/shadow-event-path-expected.txt	(revision 235806)
>+++ LayoutTests/fast/events/shadow-event-path-expected.txt	(working copy)
>@@ -143,6 +143,30 @@ mouseleave@input#target
>     target:input#target
>     relatedTarget:div#divInsideSummary
> 
>+mouseleave@div#divInsideSummary
>+    target:div#divInsideSummary
>+    relatedTarget:div#divInsideSummary
>+
>+mouseleave@summary
>+    target:summary
>+    relatedTarget:div#divInsideSummary
>+
>+mouseleave@details
>+    target:details
>+    relatedTarget:div#divInsideSummary
>+
>+mouseleave@div#detailsContainer
>+    target:div#detailsContainer
>+    relatedTarget:div#divInsideSummary
>+
>+mouseleave@body
>+    target:body
>+    relatedTarget:div#divInsideSummary
>+
>+mouseleave@html
>+    target:html
>+    relatedTarget:div#divInsideSummary
>+
> mouseover@div#divInsideSummary
>     target:div#divInsideSummary
>     relatedTarget:input#target
>@@ -175,6 +199,30 @@ mouseover@window
>     target:div#divInsideSummary
>     relatedTarget:input#target
> 
>+mouseenter@div#divInsideSummary
>+    target:div#divInsideSummary
>+    relatedTarget:input#target
>+
>+mouseenter@summary
>+    target:summary
>+    relatedTarget:input#target
>+
>+mouseenter@details
>+    target:details
>+    relatedTarget:input#target
>+
>+mouseenter@div#detailsContainer
>+    target:div#detailsContainer
>+    relatedTarget:input#target
>+
>+mouseenter@body
>+    target:body
>+    relatedTarget:input#target
>+
>+mouseenter@html
>+    target:html
>+    relatedTarget:input#target
>+
> mousemove@div#divInsideSummary
>     target:div#divInsideSummary
>     relatedTarget:null
>Index: LayoutTests/fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary-expected.txt
>===================================================================
>--- LayoutTests/fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary-expected.txt	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary-expected.txt	(working copy)
>@@ -0,0 +1,28 @@
>+This tests mouseenter and mouseleave events across shadow boundaries.
>+
>+
>+==Entering host parent==
>+mouseenter on hostParent
>+
>+==Entering host==
>+mouseenter on host
>+
>+==Entering shadow content==
>+mouseenter on slotParent
>+
>+==Entering slotted text==
>+mouseenter on slot
>+
>+==Entering slotted element==
>+mouseenter on target
>+mouseenter on targetParent
>+
>+==Leaving slotted element==
>+mouseleave on target
>+mouseleave on targetParent
>+mouseleave on slot
>+
>+==Leaving host==
>+mouseleave on slotParent
>+mouseleave on host
>+
>Index: LayoutTests/fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary.html
>===================================================================
>--- LayoutTests/fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary.html	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/mouseenter-mouseleave-across-shadow-boundary.html	(working copy)
>@@ -0,0 +1,68 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This tests mouseenter and mouseleave events across shadow boundaries.</p>
>+<div id="hostParent" style="display: inline-block; border: solid 10px green;">
>+<div id="host" style="display: inline-block;">
>+    text content
>+    <p id="targetParent" style="margin: 0;"><span id="target">element content</span></p>
>+</div></div><pre id="log"></pre>
>+<script>
>+
>+const host = document.getElementById('host');
>+const shadowRoot = host.attachShadow({mode: 'closed'});
>+shadowRoot.innerHTML = `
>+    <div id="slotParent" style="width: 200px; margin: 10px; padding: 10px; border: solid 10px blue;">
>+        <slot id="slot"></slot>
>+    </div>`;
>+const slot = shadowRoot.querySelector('slot');
>+
>+const target = document.getElementById('target');
>+for (const element of [host.parentNode, host, target, target.parentNode, slot, slot.parentNode]) {
>+    element.addEventListener('mouseenter', logEvent);
>+    element.addEventListener('mouseleave', logEvent);
>+}
>+
>+function log(text) {
>+    document.getElementById('log').textContent += text + '\n';
>+}
>+
>+function logEvent(event) {
>+    log(`${event.type} on ${event.target.id}`);
>+}
>+
>+function logPhase(phase) {
>+    log(`\n==${phase}==`);
>+}
>+
>+if (!window.eventSender)
>+    document.write('This test requires eventSender');
>+else {
>+    testRunner.dumpAsText();
>+    const hostRect = host.getBoundingClientRect();
>+    const targetRect = target.getBoundingClientRect();
>+    const xAboveText = hostRect.x + 40;
>+    let y = hostRect.top;
>+    logPhase('Entering host parent');
>+    eventSender.mouseMoveTo(xAboveText, y - 5);
>+    logPhase('Entering host');
>+    eventSender.mouseMoveTo(xAboveText, y + 5);
>+    logPhase('Entering shadow content');
>+    eventSender.mouseMoveTo(xAboveText, y + 15);
>+
>+    logPhase('Entering slotted text');
>+    eventSender.mouseMoveTo(xAboveText, targetRect.top - 5);
>+    logPhase('Entering slotted element');
>+    eventSender.mouseMoveTo(xAboveText, targetRect.top + 5);
>+
>+    logPhase('Leaving slotted element');
>+    eventSender.mouseMoveTo(xAboveText, targetRect.bottom + 5);
>+    logPhase('Leaving host');
>+    eventSender.mouseMoveTo(xAboveText, hostRect.bottom + 5);
>+    
>+    host.parentNode.style.display = 'none';
>+}
>+
>+</script>
>+</body>
>+</html>
>Index: LayoutTests/fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree-expected.txt
>===================================================================
>--- LayoutTests/fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree-expected.txt	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree-expected.txt	(working copy)
>@@ -0,0 +1,17 @@
>+This tests mouseenter and mouseleave events inside a shadow tree.
>+To manually test, move the mouse cursor vertically across "hello, world" below.
>+WebKit must generate mouseenter and mouseleve events exactly once on host (as well as hostParent, continer, and target).
>+
>+
>+==Entering target==
>+mouseenter on target
>+mouseenter on container
>+mouseenter on host
>+mouseenter on hostParent
>+
>+==Leaving target==
>+mouseleave on target
>+mouseleave on container
>+mouseleave on host
>+mouseleave on hostParent
>+
>Index: LayoutTests/fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree.html
>===================================================================
>--- LayoutTests/fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree.html	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/mouseenter-mouseleave-inside-shadow-tree.html	(working copy)
>@@ -0,0 +1,54 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This tests mouseenter and mouseleave events inside a shadow tree.<br>
>+To manually test, move the mouse cursor vertically across "hello, world" below.<br>
>+WebKit must generate mouseenter and mouseleve events exactly once on host (as well as hostParent, continer, and target).</p>
>+<div id="hostParent"><div id="host"></div></div><pre id="log"></pre>
>+<script>
>+
>+const host = document.getElementById('host');
>+const shadowRoot = host.attachShadow({mode: 'closed'});
>+shadowRoot.innerHTML = `
>+    <div id="container">
>+        <span id="target">hello, world</span>
>+    </div>`;
>+const container = shadowRoot.getElementById('container');
>+const target = shadowRoot.getElementById('target');
>+
>+for (const element of [host.parentNode, host, container, target]) {
>+    element.addEventListener('mouseenter', logEvent);
>+    element.addEventListener('mouseleave', logEvent);
>+}
>+
>+function log(text) {
>+    document.getElementById('log').textContent += text + '\n';
>+}
>+
>+function logEvent(event) {
>+    if (event.composed)
>+        log(`FAIL - ${event.type} on ${event.target.id} was composed`);
>+    log(`${event.type} on ${event.target.id}`);
>+}
>+
>+function logPhase(phase) {
>+    log(`\n==${phase}==`);
>+}
>+
>+if (!window.eventSender)
>+    document.write('This test requires eventSender');
>+else {
>+    testRunner.dumpAsText();
>+    const targetRect = target.getBoundingClientRect();
>+    const x = targetRect.x + 5;
>+    logPhase('Entering target');
>+    eventSender.mouseMoveTo(x, targetRect.top + 5);
>+    logPhase('Leaving target');
>+    eventSender.mouseMoveTo(x, targetRect.bottom + 5);
>+
>+    host.parentNode.style.display = 'none';
>+}
>+
>+</script>
>+</body>
>+</html>
>Index: LayoutTests/fast/shadow-dom/mouseenter-mouseleave-on-slot-parent-expected.txt
>===================================================================
>--- LayoutTests/fast/shadow-dom/mouseenter-mouseleave-on-slot-parent-expected.txt	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/mouseenter-mouseleave-on-slot-parent-expected.txt	(working copy)
>@@ -0,0 +1,15 @@
>+This tests mouseenter and mouseleave event fires when capturing event listeners are only present on the slot element's parent.
>+To manually test, move the mouse cursor into the box below and move it out.
>+You should see mouseenter events on slotContainer, slot, and target in that order and mouseleave events in the reverse order.
>+
>+
>+==Entering target==
>+mouseenter on target
>+mouseenter on slot
>+mouseenter on slotContainer
>+
>+==Leaving target==
>+mouseleave on target
>+mouseleave on slot
>+mouseleave on slotContainer
>+
>Index: LayoutTests/fast/shadow-dom/mouseenter-mouseleave-on-slot-parent.html
>===================================================================
>--- LayoutTests/fast/shadow-dom/mouseenter-mouseleave-on-slot-parent.html	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/mouseenter-mouseleave-on-slot-parent.html	(working copy)
>@@ -0,0 +1,51 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This tests mouseenter and mouseleave event fires when capturing event listeners are only present on the slot element's parent.<br>
>+To manually test, move the mouse cursor into the box below and move it out.<br>
>+You should see mouseenter events on slotContainer, slot, and target in that order and mouseleave events in the reverse order.</p>
>+<div id="hostParent"><div id="host"><span id="target" style="border: solid 1px black; padding: 0.5rem;">content</span></div></div><pre id="log"></pre>
>+<script>
>+
>+const host = document.getElementById('host');
>+const shadowRoot = host.attachShadow({mode: 'closed'});
>+shadowRoot.innerHTML = `
>+    <div id="slotContainer">
>+        <slot id="slot"></slot>
>+    </div>`;
>+const slot = shadowRoot.querySelector('slot');
>+
>+const target = document.getElementById('target');
>+slot.parentNode.addEventListener('mouseenter', logEvent, true);
>+slot.parentNode.addEventListener('mouseleave', logEvent, true);
>+
>+function log(text) {
>+    document.getElementById('log').textContent += text + '\n';
>+}
>+
>+function logEvent(event) {
>+    log(`${event.type} on ${event.target.id}`);
>+}
>+
>+function logPhase(phase) {
>+    log(`\n==${phase}==`);
>+}
>+
>+if (!window.eventSender)
>+    document.write('This test requires eventSender');
>+else {
>+    testRunner.dumpAsText();
>+    const targetRect = target.getBoundingClientRect();
>+    const x = targetRect.x + 40;
>+    logPhase('Entering target');
>+    eventSender.mouseMoveTo(x, targetRect.top + 5);
>+
>+    logPhase('Leaving target');
>+    eventSender.mouseMoveTo(x, targetRect.bottom + 5);
>+    
>+    host.parentNode.style.display = 'none';
>+}
>+
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 188561: 347090 | 349229 | 349234 | 349236 | 349237 | 349242