Revert "Add the beginning of hover-trigger for pop-ups"

This reverts commit cdbdb4520d6be65fdb04fc85aea5d12953f1a1da. Reason for revert: Suspected cause of 1344436 Original change's description: > Add the beginning of hover-trigger for pop-ups > > This CL adds the 'hoverpopup' attribute, to go along with the other > invoking attributes (togglepopup, showpopup, hidepopup). This attribute, > when places on *any* element, triggers a pop-up when hovered. With this > CL, it is always triggered after a fixed 100ms delay, but that will be > made configurable via CSS in subsequent CLs. There is also no "hide" > trigger when any element is de-hovered - again that will be added > later. > > See this spec discussion: > openui/open-ui#526 (comment) > > Bug: 1307772 > Change-Id: I7af88dad9efa015f833843ea76bed41b4aa42c4b > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3759184 > Reviewed-by: David Baron <[email protected]> > Auto-Submit: Mason Freed <[email protected]> > Commit-Queue: Mason Freed <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1024141} Bug: 1307772, 1344436 Change-Id: I65a1e36ead974e887c37b6a90ff6504fdaa45d71 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3760676 Bot-Commit: Rubber Stamper <[email protected]> Commit-Queue: Theodore Olsauskas-Warren <[email protected]> Owners-Override: Theodore Olsauskas-Warren <[email protected]> Cr-Commit-Position: refs/heads/main@{#1024173} NOKEYCHECK=True GitOrigin-RevId: ddfbc99283fdb659310ac4261a9de0a64e77d7b5
mjfroman · Jul 14, 2022 · 1d8e4d4 · 1d8e4d4
1 parent f63f1f2
commit 1d8e4d4
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 170 deletions.
diff --git a/blink/renderer/core/dom/element.cc b/blink/renderer/core/dom/element.cc
@@ -3061,60 +3061,6 @@ Element* Element::anchorElement() const {
   return GetTreeScope().getElementById(anchor_id);  // may be null
 }
 
-void Element::MaybeTriggerHoverPopup(Element* popup_element) {
-  DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
-  if (!popup_element || !popup_element->HasValidPopupAttribute())
-    return;
-  // Remove this element from hoverPopupTasks always.
-  popup_element->GetPopupData()->hoverPopupTasks().erase(this);
-  // Only trigger the pop-up if the hoverpopup attribute still points to the
-  // same pop-up, and the pop-up is in the tree and still not showing.
-  if (popup_element->IsInTreeScope() && !popup_element->popupOpen() &&
-      popup_element == GetTreeScope().getElementById(
-                           FastGetAttribute(html_names::kHoverpopupAttr))) {
-    popup_element->showPopUp(ASSERT_NO_EXCEPTION);
-  }
-}
-
-void Element::HandlePopupHovered(bool hovered) {
-  if (!RuntimeEnabledFeatures::HTMLPopupAttributeEnabled())
-    return;
-  if (!FastHasAttribute(html_names::kHoverpopupAttr) || !IsInTreeScope())
-    return;
-  Element* popup_element = GetTreeScope().getElementById(
-      FastGetAttribute(html_names::kHoverpopupAttr));
-  if (!popup_element || !popup_element->HasValidPopupAttribute())
-    return;
-  if (hovered) {
-    auto& hover_tasks = popup_element->GetPopupData()->hoverPopupTasks();
-    DCHECK(!hover_tasks.Contains(this));
-
-    // TODO(masonf): Use CSS timeout value instead of this hard-coded timeout.
-    constexpr base::TimeDelta kDelayBeforeShow = base::Milliseconds(100);
-
-    // When we enter an element, we'll post a delayed task for the pop-up we're
-    // targeting. It's possible that multiple nested elements have hoverpopup
-    // attributes pointing to the same pop-up, and in that case, we want to
-    // trigger on the first of them that reaches its timeout threshold.
-    hover_tasks.insert(
-        this,
-        PostDelayedCancellableTask(
-            *GetExecutionContext()->GetTaskRunner(TaskType::kInternalDefault),
-            FROM_HERE,
-            WTF::Bind(&Element::MaybeTriggerHoverPopup,
-                      WrapWeakPersistent(this),
-                      WrapWeakPersistent(popup_element)),
-            kDelayBeforeShow));
-  } else {
-    // If we have a task still waiting, cancel it.
-    popup_element->GetPopupData()->hoverPopupTasks().Take(this).Cancel();
-    // TODO(masonf): Still need to implement the code to hide this pop-up after
-    // a configurable delay. That needs to work even if the pop-up wasn't
-    // triggered by a hoverpopup attribute. E.g. a regular pop-up that gets
-    // hidden after it has not been hovered for n seconds.
-  }
-}
-
 void Element::SetNeedsRepositioningForSelectMenu(bool flag) {
   DCHECK(RuntimeEnabledFeatures::HTMLSelectMenuElementEnabled());
   DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
@@ -8298,7 +8244,6 @@ void Element::SetHovered(bool hovered) {
     return;
 
   GetDocument().UserActionElements().SetHovered(this, hovered);
-  HandlePopupHovered(hovered);
 
   const ComputedStyle* style = GetComputedStyle();
   if (!style || style->AffectedByHover()) {

diff --git a/blink/renderer/core/dom/element.h b/blink/renderer/core/dom/element.h
@@ -594,8 +594,6 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
                                  HidePopupFocusBehavior,
                                  HidePopupForcingLevel,
                                  HidePopupIndependence);
-  void MaybeTriggerHoverPopup(Element* popup_element);
-  void HandlePopupHovered(bool hovered);
 
   // TODO(crbug.com/1197720): The popup position should be provided by the new
   // anchored positioning scheme.

diff --git a/blink/renderer/core/dom/popup_data.h b/blink/renderer/core/dom/popup_data.h
@@ -81,10 +81,6 @@ class PopupData final : public GarbageCollected<PopupData> {
     animation_finished_listener_ = listener;
   }
 
-  HeapHashMap<WeakMember<Element>, TaskHandle>& hoverPopupTasks() {
-    return hover_popup_tasks_;
-  }
-
   HTMLSelectMenuElement* ownerSelectMenuElement() const {
     return owner_select_menu_element_;
   }
@@ -96,7 +92,6 @@ class PopupData final : public GarbageCollected<PopupData> {
     visitor->Trace(invoker_);
     visitor->Trace(previously_focused_element_);
     visitor->Trace(animation_finished_listener_);
-    visitor->Trace(hover_popup_tasks_);
     visitor->Trace(owner_select_menu_element_);
   }
 
@@ -110,9 +105,6 @@ class PopupData final : public GarbageCollected<PopupData> {
   // We hold a strong reference to the animation finished listener, so that we
   // can confirm that the listeners get removed before cleanup.
   Member<PopupAnimationFinishedEventListener> animation_finished_listener_;
-  // Map from triggering elements to a TaskHandle for the task that will invoke
-  // the pop-up.
-  HeapHashMap<WeakMember<Element>, TaskHandle> hover_popup_tasks_;
 
   // TODO(crbug.com/1197720): The popup position should be provided by the new
   // anchored positioning scheme.

diff --git a/blink/renderer/core/html/html_attribute_names.json5 b/blink/renderer/core/html/html_attribute_names.json5
@@ -102,7 +102,6 @@
     "hidden",
     "hidepopup",
     "high",
-    "hoverpopup",
     "href",
     "hreflang",
     "hreftranslate",

diff --git a/blink/renderer/core/html/html_element.cc b/blink/renderer/core/html/html_element.cc
@@ -54,7 +54,6 @@
 #include "third_party/blink/renderer/core/editing/editing_utilities.h"
 #include "third_party/blink/renderer/core/editing/serializers/serialization.h"
 #include "third_party/blink/renderer/core/editing/spellcheck/spell_checker.h"
-#include "third_party/blink/renderer/core/event_type_names.h"
 #include "third_party/blink/renderer/core/events/keyboard_event.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
@@ -443,23 +442,20 @@ AttributeTriggers* HTMLElement::TriggersForAttributeName(
   static AttributeTriggers attribute_triggers[] = {
       {html_names::kDirAttr, kNoWebFeature, kNoEvent,
        &HTMLElement::OnDirAttrChanged},
+      {html_names::kFocusgroupAttr, kNoWebFeature, kNoEvent,
+       &HTMLElement::OnFocusgroupAttrChanged},
       {html_names::kFormAttr, kNoWebFeature, kNoEvent,
        &HTMLElement::OnFormAttrChanged},
       {html_names::kLangAttr, kNoWebFeature, kNoEvent,
        &HTMLElement::OnLangAttrChanged},
       {html_names::kNonceAttr, kNoWebFeature, kNoEvent,
        &HTMLElement::OnNonceAttrChanged},
-
-      {html_names::kFocusgroupAttr, kNoWebFeature, kNoEvent,
-       &HTMLElement::ReparseAttribute},
       {html_names::kTabindexAttr, kNoWebFeature, kNoEvent,
-       &HTMLElement::ReparseAttribute},
+       &HTMLElement::OnTabIndexAttrChanged},
       {xml_names::kLangAttr, kNoWebFeature, kNoEvent,
-       &HTMLElement::ReparseAttribute},
+       &HTMLElement::OnXMLLangAttrChanged},
       {html_names::kPopupAttr, kNoWebFeature, kNoEvent,
-       &HTMLElement::ReparseAttribute},
-      {html_names::kHoverpopupAttr, kNoWebFeature, kNoEvent,
-       &HTMLElement::ReparseAttribute},
+       &HTMLElement::OnPopupAttrChanged},
 
       {html_names::kOnabortAttr, kNoWebFeature, event_type_names::kAbort,
        nullptr},
@@ -2025,7 +2021,8 @@ void HTMLElement::OnDirAttrChanged(const AttributeModificationParams& params) {
   }
 }
 
-void HTMLElement::ReparseAttribute(const AttributeModificationParams& params) {
+void HTMLElement::OnFocusgroupAttrChanged(
+    const AttributeModificationParams& params) {
   Element::ParseAttribute(params);
 }
 
@@ -2044,6 +2041,21 @@ void HTMLElement::OnNonceAttrChanged(
     setNonce(params.new_value);
 }
 
+void HTMLElement::OnTabIndexAttrChanged(
+    const AttributeModificationParams& params) {
+  Element::ParseAttribute(params);
+}
+
+void HTMLElement::OnXMLLangAttrChanged(
+    const AttributeModificationParams& params) {
+  Element::ParseAttribute(params);
+}
+
+void HTMLElement::OnPopupAttrChanged(
+    const AttributeModificationParams& params) {
+  Element::ParseAttribute(params);
+}
+
 ElementInternals* HTMLElement::attachInternals(
     ExceptionState& exception_state) {
   // 1. If this's is value is not null, then throw a "NotSupportedError"

diff --git a/blink/renderer/core/html/html_element.h b/blink/renderer/core/html/html_element.h
@@ -264,11 +264,13 @@ class CORE_EXPORT HTMLElement : public Element {
       const QualifiedName& attr_name);
 
   void OnDirAttrChanged(const AttributeModificationParams&);
+  void OnFocusgroupAttrChanged(const AttributeModificationParams&);
   void OnFormAttrChanged(const AttributeModificationParams&);
   void OnLangAttrChanged(const AttributeModificationParams&);
   void OnNonceAttrChanged(const AttributeModificationParams&);
-
-  void ReparseAttribute(const AttributeModificationParams&);
+  void OnTabIndexAttrChanged(const AttributeModificationParams&);
+  void OnXMLLangAttrChanged(const AttributeModificationParams&);
+  void OnPopupAttrChanged(const AttributeModificationParams&);
 };
 
 template <typename T>

diff --git a/blink/web_tests/external/wpt/html/semantics/popups/popup-hoverpopup-attribute.tentative.html b/blink/web_tests/external/wpt/html/semantics/popups/popup-hoverpopup-attribute.tentative.html