Update hover states when dragging the mouse into an element

third_party/WebKit/Source/core/dom/Document.cpp

Index: third_party/WebKit/Source/core/dom/Document.cpp
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index 1237ded036ba45e531b0417259a5a5afc84cd3b8..f9ba4b6b94b2f5594253506eb3c040450fb7f2ce 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -6507,7 +6507,7 @@ void Document::UpdateHoverActiveState(const HitTestRequest& request,
   bool allow_active_changes = !old_active_element && ActiveHoverElement();
 
   // If the mouse is down and if this is a mouse move event, we want to restrict
-  // changes in :hover/:active to only apply to elements that are in the :active
+  // changes in :active to only apply to elements that are in the :active
   // chain that we froze at the time the mouse went down.
   bool must_be_in_active_chain = request.Active() && request.Move();
 
@@ -6534,7 +6534,8 @@ void Document::UpdateHoverActiveState(const HitTestRequest& request,
   }
 
   HeapVector<Member<Element>, 32> elements_to_remove_from_chain;

[mustaq, 2017/07/10 15:40:45]

This asymmetric list for removal vs addition give me some worries.  Why we
remove w/o worrying about active/hover distinction but care about the
distinction when adding?

[lanwei, 2017/07/12 21:03:10]

Done.

-  HeapVector<Member<Element>, 32> elements_to_add_to_chain;
+  HeapVector<Member<Element>, 32> elements_to_add_to_active_chain;
+  HeapVector<Member<Element>, 32> elements_to_add_to_hover_chain;
 
   if (old_hover_element != new_hover_element) {
     // The old hover path only needs to be cleared up to (and not including) the
@@ -6544,14 +6545,10 @@ void Document::UpdateHoverActiveState(const HitTestRequest& request,
     // tree already. This is due to our handling of m_hoverElement in
     // hoveredElementDetached (which assumes all the parents are hovered) and
     // mustBeInActiveChain (which makes this not hold).

[Navid Zolghadr, 2017/07/07 18:21:00]

nit: I believe this comment also doesn't hold since it talks about
mustBeInActiveChain and you have removed it.

[lanwei, 2017/07/12 21:03:10]

Done.

-    //
-    // In that case, none of the nodes in the chain have the flags, so there's
-    // no problem in skipping this step.
     if (old_hover_element && old_hover_element->isConnected()) {
       for (Element* curr = old_hover_element; curr && curr != ancestor_element;
            curr = FlatTreeTraversal::ParentElement(*curr)) {
-        if (!must_be_in_active_chain || curr->InActiveChain())
-          elements_to_remove_from_chain.push_back(curr);
+        elements_to_remove_from_chain.push_back(curr);
       }
     }
   }
@@ -6559,8 +6556,10 @@ void Document::UpdateHoverActiveState(const HitTestRequest& request,
   // Now set the hover state for our new object up to the root.
   for (Element* curr = new_hover_element; curr;
        curr = FlatTreeTraversal::ParentElement(*curr)) {
+    if (old_hover_element != new_hover_element)
+      elements_to_add_to_hover_chain.push_back(curr);
     if (!must_be_in_active_chain || curr->InActiveChain())
-      elements_to_add_to_chain.push_back(curr);
+      elements_to_add_to_active_chain.push_back(curr);
   }
 
   if (allow_hover_changes) {
@@ -6568,14 +6567,17 @@ void Document::UpdateHoverActiveState(const HitTestRequest& request,
       element->SetHovered(false);
   }
 
+  // Elements past the common ancestor do not change hover state, but might
+  // change active state.
+  for (Element* element : elements_to_add_to_active_chain) {
+    if (allow_active_changes)
+      element->SetActive(true);
+  }
+
   bool saw_common_ancestor = false;
-  for (Element* element : elements_to_add_to_chain) {
-    // Elements past the common ancestor do not change hover state, but might
-    // change active state.
+  for (Element* element : elements_to_add_to_hover_chain) {
     if (element == ancestor_element)
       saw_common_ancestor = true;
-    if (allow_active_changes)
-      element->SetActive(true);
     if (allow_hover_changes &&
         (!saw_common_ancestor || element == hover_element_))
       element->SetHovered(true);