Skip to content

Commit

Permalink
Remove hover-triggered behaviors from the Pop-Up API
Browse files Browse the repository at this point in the history 
Rather than keeping all of this complexity in the codebase, based
on the decision [1] to "punt" this behavior to V2 of the API, I'm
going to remove it all. This CL can be reverted to bring it back.

This CL mostly reverts the CLs below, but some functionality is
left in place, such as the CSS time interpolation code. But for
the most part, this reverts these CLs:

 1. https://chromium-review.googlesource.com/c/chromium/src/+/3763422
 2. https://chromium-review.googlesource.com/c/chromium/src/+/3760885
 3. https://chromium-review.googlesource.com/c/chromium/src/+/3781406
 4. https://chromium-review.googlesource.com/c/chromium/src/+/3812260
 5. https://chromium-review.googlesource.com/c/chromium/src/+/3811136

[1] openui/open-ui#526

Bug: 1307772
Change-Id: I6c8e4c9d2cf1ef0d36df71767b26122f65ea5b5b
Fixed: 1349910
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3842343
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1037917}
NOKEYCHECK=True
GitOrigin-RevId: d2c596ab992d3bbe287a184e2e518354cfa37e40
  • Loading branch information
@mfreed7 @copybara-github
mfreed7 authored and copybara-github committed Aug 22, 2022
1 parent 9d0004c commit 7bf8d3b
Show file tree
Hide file tree
Showing 20 changed files with 8 additions and 662 deletions.
5 changes: 0 additions & 5 deletions blink/renderer/core/animation/css_interpolation_types_map.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,6 @@ const InterpolationTypes& CSSInterpolationTypesMap::Get(
applicable_types->push_back(
std::make_unique<CSSNumberInterpolationType>(used_property));
break;
case CSSPropertyID::kPopUpShowDelay:
case CSSPropertyID::kPopUpHideDelay:
applicable_types->push_back(
std::make_unique<CSSTimeInterpolationType>(used_property));
break;
case CSSPropertyID::kAccentColor:
case CSSPropertyID::kBackgroundColor:
case CSSPropertyID::kBorderBottomColor:
Expand Down
19 changes: 3 additions & 16 deletions blink/renderer/core/animation/css_time_interpolation_type.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ absl::optional<double> CSSTimeInterpolationType::GetSeconds(
const CSSPropertyID& property,
const ComputedStyle& style) {
switch (property) {
case CSSPropertyID::kPopUpShowDelay:
return style.PopUpShowDelay();
case CSSPropertyID::kPopUpHideDelay:
return style.PopUpHideDelay();
// No properties currently use CSSTimeInterpolationType.
default:
NOTREACHED();
return absl::optional<double>();
Expand All @@ -70,9 +67,7 @@ absl::optional<double> CSSTimeInterpolationType::GetSeconds(
double CSSTimeInterpolationType::ClampTime(const CSSPropertyID& property,
double value) const {
switch (property) {
case CSSPropertyID::kPopUpShowDelay:
case CSSPropertyID::kPopUpHideDelay:
return ClampTo<float>(value, 0);
// No properties currently use CSSTimeInterpolationType.
default:
NOTREACHED();
return 0;
Expand All @@ -92,16 +87,8 @@ void CSSTimeInterpolationType::ApplyStandardPropertyValue(
const NonInterpolableValue*,
StyleResolverState& state) const {
auto property = CssProperty().PropertyID();
double clamped_seconds =
ClampTime(property, To<InterpolableNumber>(interpolable_value).Value());
ComputedStyle& style = *state.Style();
switch (property) {
case CSSPropertyID::kPopUpShowDelay:
style.SetPopUpShowDelay(clamped_seconds);
break;
case CSSPropertyID::kPopUpHideDelay:
style.SetPopUpHideDelay(clamped_seconds);
break;
// No properties currently use CSSTimeInterpolationType.
default:
NOTREACHED();
break;
Expand Down
26 changes: 0 additions & 26 deletions blink/renderer/core/css/css_properties.json5
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2814,32 +2814,6 @@
valid_for_canvas_formatted_text: true,
valid_for_position_fallback: true,
},
{
name: "pop-up-show-delay",
runtime_flag: "HTMLPopupAttribute",
property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
field_group: "*",
field_template: "primitive",
type_name: "float",
interpolable: true,
typedom_types: ["Time"],
converter: "ConvertTimeValue",
default_value: "0.5",
supports_incremental_style: true,
},
{
name: "pop-up-hide-delay",
runtime_flag: "HTMLPopupAttribute",
property_methods: ["ParseSingleValue", "CSSValueFromComputedStyleInternal"],
field_group: "*",
field_template: "primitive",
type_name: "float",
interpolable: true,
typedom_types: ["Time"],
converter: "ConvertTimeValue",
default_value: "HUGE_VALF",
supports_incremental_style: true,
},
{
name: "hyphens",
property_methods: ["CSSValueFromComputedStyleInternal"],
Expand Down
4 changes: 0 additions & 4 deletions blink/renderer/core/css/css_property_equality.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,6 @@ bool CSSPropertyEquality::PropertiesEqual(const PropertyHandle& property,
return a.GridTemplateRows() == b.GridTemplateRows();
case CSSPropertyID::kHeight:
return a.Height() == b.Height();
case CSSPropertyID::kPopUpShowDelay:
return a.PopUpShowDelay() == b.PopUpShowDelay();
case CSSPropertyID::kPopUpHideDelay:
return a.PopUpHideDelay() == b.PopUpHideDelay();
case CSSPropertyID::kLeft:
return a.Left() == b.Left();
case CSSPropertyID::kLetterSpacing:
Expand Down
32 changes: 0 additions & 32 deletions blink/renderer/core/css/properties/longhands/longhands_custom.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3645,38 +3645,6 @@ const CSSValue* Height::CSSValueFromComputedStyleInternal(
style);
}

const CSSValue* PopUpShowDelay::ParseSingleValue(
CSSParserTokenRange& range,
const CSSParserContext& context,
const CSSParserLocalContext& local_context) const {
return css_parsing_utils::ConsumeTime(
range, context, CSSPrimitiveValue::ValueRange::kNonNegative);
}

const CSSValue* PopUpShowDelay::CSSValueFromComputedStyleInternal(
const ComputedStyle& style,
const LayoutObject* layout_object,
bool allow_visited_style) const {
return CSSNumericLiteralValue::Create(style.PopUpShowDelay(),
CSSPrimitiveValue::UnitType::kSeconds);
}

const CSSValue* PopUpHideDelay::ParseSingleValue(
CSSParserTokenRange& range,
const CSSParserContext& context,
const CSSParserLocalContext& local_context) const {
return css_parsing_utils::ConsumeTime(
range, context, CSSPrimitiveValue::ValueRange::kNonNegative);
}

const CSSValue* PopUpHideDelay::CSSValueFromComputedStyleInternal(
const ComputedStyle& style,
const LayoutObject* layout_object,
bool allow_visited_style) const {
return CSSNumericLiteralValue::Create(style.PopUpHideDelay(),
CSSPrimitiveValue::UnitType::kSeconds);
}

const CSSValue* Hyphens::CSSValueFromComputedStyleInternal(
const ComputedStyle& style,
const LayoutObject*,
Expand Down
2 changes: 0 additions & 2 deletions blink/renderer/core/dom/document.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4842,7 +4842,6 @@ void Document::DynamicViewportUnitsChanged() {
}

void Document::SetHoverElement(Element* new_hover_element) {
Element::HoveredElementChanged(hover_element_, new_hover_element);
hover_element_ = new_hover_element;
}

Expand Down Expand Up @@ -8198,7 +8197,6 @@ void Document::Trace(Visitor* visitor) const {
visitor->Trace(popup_stack_);
visitor->Trace(pop_up_mousedown_target_);
visitor->Trace(popups_waiting_to_hide_);
visitor->Trace(all_open_pop_ups_);
visitor->Trace(elements_needing_style_recalc_for_toggle_);
visitor->Trace(load_event_delay_timer_);
visitor->Trace(plugin_loading_timer_);
Expand Down
3 changes: 0 additions & 3 deletions blink/renderer/core/dom/document.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,6 @@ class CORE_EXPORT Document : public ContainerNode,
HeapVector<Member<Element>>& PopupStack() { return popup_stack_; }
const HeapVector<Member<Element>>& PopupStack() const { return popup_stack_; }
bool PopupAutoShowing() const { return !popup_stack_.IsEmpty(); }
HeapHashSet<Member<Element>>& AllOpenPopUps() { return all_open_pop_ups_; }
Element* TopmostPopupAutoOrHint() const;
HeapHashSet<Member<Element>>& PopupsWaitingToHide() {
return popups_waiting_to_hide_;
Expand Down Expand Up @@ -2353,8 +2352,6 @@ class CORE_EXPORT Document : public ContainerNode,
// A set of popups for which hidePopUp() has been called, but animations are
// still running.
HeapHashSet<Member<Element>> popups_waiting_to_hide_;
// A set of all open pop-ups, of all types.
HeapHashSet<Member<Element>> all_open_pop_ups_;

// Elements that need to be restyled because a toggle was created on them,
// or a prior sibling, during the previous restyle.
Expand Down
187 changes: 0 additions & 187 deletions blink/renderer/core/dom/element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2574,8 +2574,6 @@ void Element::showPopUp(ExceptionState& exception_state) {
document.SetPopupHintShowing(this);
}
}
DCHECK(!document.AllOpenPopUps().Contains(this));
document.AllOpenPopUps().insert(this);

GetPopupData()->setAnimationFinishedListener(nullptr);
GetPopupData()->setPreviouslyFocusedElement(
Expand All @@ -2595,12 +2593,6 @@ void Element::showPopUp(ExceptionState& exception_state) {
GetPopupData()->setVisibilityState(PopupVisibilityState::kShowing);
PseudoStateChanged(CSSSelector::kPseudoTopLayer);

// Queue a delayed hide event, if necessary.
if (!GetDocument().HoverElement() ||
!IsNodePopUpDescendant(*GetDocument().HoverElement())) {
MaybeQueuePopupHideEvent();
}

SetPopupFocusOnShow();
}

Expand Down Expand Up @@ -2734,7 +2726,6 @@ void Element::HidePopUpInternal(HidePopupFocusBehavior focus_behavior,
document.SetPopupHintShowing(nullptr);
}
}
document.AllOpenPopUps().erase(this);
document.PopupsWaitingToHide().insert(this);

bool force_hide = forcing_level == HidePopupForcingLevel::kHideImmediately;
Expand Down Expand Up @@ -2929,8 +2920,6 @@ const Element* NearestOpenAncestralPopupRecursive(
if (auto* form_control = DynamicTo<HTMLFormControlElement>(element)) {
recurse_and_update(form_control->popupTargetElement().element);
}
if (auto* hover_popup_element = element->PopupHoverTargetElement())
recurse_and_update(hover_popup_element);
// Include the anchor elements for all showing pop-ups.
if (anchors_to_popups.Contains(element)) {
recurse_and_update(anchors_to_popups.at(element));
Expand Down Expand Up @@ -3107,181 +3096,6 @@ Element* Element::anchorElement() const {
return GetTreeScope().getElementById(anchor_id); // may be null
}

Element* Element::PopupHoverTargetElement() const {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled(
GetDocument().GetExecutionContext()));
if (!FastHasAttribute(html_names::kPopuphovertargetAttr))
return nullptr;
Element* popup_element = GetTreeScope().getElementById(
FastGetAttribute(html_names::kPopuphovertargetAttr));
if (!popup_element || !popup_element->HasValidPopupAttribute())
return nullptr;
return popup_element;
}

// Must be called on an Element that is a pop-up. Returns true if |node| is a
// descendant of this pop-up. This includes the case where |node| is contained
// within another pop-up, and the container pop-up is a descendant of this
// pop_up. For the special case of popup=manual pop-ups, which do not have
// ancestral relationships, this function checks pure DOM tree descendants of
// popup=manual pop-ups. This is important for the `pop-up-hide-delay` CSS
// property, which works for all pop-up types, and needs to keep pop-ups open
// when a descendant is hovered.
bool Element::IsNodePopUpDescendant(const Node& node) const {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled(
GetDocument().GetExecutionContext()));
DCHECK(HasValidPopupAttribute());
if (PopupType() == PopupValueType::kManual) {
for (const Node& ancestor : FlatTreeTraversal::InclusiveAncestorsOf(node)) {
if (ancestor == this)
return true;
}
} else {
for (const Element* ancestor =
NearestOpenAncestralPopup(node, /*inclusive*/ true);
ancestor; ancestor = NearestOpenAncestralPopup(*ancestor)) {
if (ancestor == this)
return true;
}
}
return false;
}

void Element::MaybeQueuePopupHideEvent() {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled(
GetDocument().GetExecutionContext()));
DCHECK(HasValidPopupAttribute());
// If the pop-up isn't showing, or it has an infinite PopUpHideDelay, do
// nothing.
if (GetPopupData()->visibilityState() == PopupVisibilityState::kHidden)
return;
if (!GetComputedStyle())
return;
float hide_delay_seconds = GetComputedStyle()->PopUpHideDelay();
// If the value is infinite or NaN, don't hide the pop-up.
if (!std::isfinite(hide_delay_seconds))
return;
// Queue the task to hide this pop-up.
GetPopupData()->setHoverHideTask(PostDelayedCancellableTask(
*GetExecutionContext()->GetTaskRunner(TaskType::kInternalDefault),
FROM_HERE,
WTF::Bind(
[](Element* pop_up) {
// Always remove this element from hoverShowTasks.
if (pop_up->GetPopupData())
pop_up->GetPopupData()->hoverShowTasks().erase(pop_up);
if (pop_up->popupOpen()) {
DCHECK(pop_up->IsInTreeScope());
pop_up->HidePopUpInternal(
HidePopupFocusBehavior::kFocusPreviousElement,
HidePopupForcingLevel::kHideAfterAnimations);
}
},
WrapWeakPersistent(this)),
base::Seconds(hide_delay_seconds)));
}

// static
void Element::HoveredElementChanged(Element* old_element,
Element* new_element) {
auto* document_for_ot =
old_element ? &old_element->GetDocument()
: (new_element ? &new_element->GetDocument() : nullptr);
if (!document_for_ot || !RuntimeEnabledFeatures::HTMLPopupAttributeEnabled(
document_for_ot->GetExecutionContext()))
return;
if (old_element) {
// For the previously-hovered element: loop through all showing popups
// (including popup=manual) and see if the element that just lost focus was
// an ancestor. If so, queue a task to hide it after a delay.
for (auto& pop_up : old_element->GetDocument().AllOpenPopUps()) {
if (pop_up->IsNodePopUpDescendant(*old_element))
pop_up->MaybeQueuePopupHideEvent();
}
}
// It is possible that both old_element and new_element are descendants of
// the same open pop_up, in which case we'll queue a hide task and then
// immediately cancel it, resulting in no change.
if (new_element) {
// For the newly-hovered element: loop through all showing popups and see if
// the newly-focused element is an ancestor. If so, cancel that pop-up's
// hide-after-delay task.
for (auto& pop_up : new_element->GetDocument().AllOpenPopUps()) {
if (pop_up->IsNodePopUpDescendant(*new_element))
pop_up->GetPopupData()->setHoverHideTask(TaskHandle());
}
}
}

void Element::HandlePopupHovered(bool hovered) {
if (!RuntimeEnabledFeatures::HTMLPopupAttributeEnabled(
GetDocument().GetExecutionContext()))
return;
if (!IsInTreeScope())
return;
if (hovered) {
// If we've just hovered an element (or the descendant of an element), see
// if it has a popuphovertarget attribute that points to a valid pop-up
// element. If so, queue a task to show the pop-up after a timeout.
Element* popup_element = PopupHoverTargetElement();
if (!popup_element)
return;
auto& hover_tasks = popup_element->GetPopupData()->hoverShowTasks();
DCHECK(!hover_tasks.Contains(this));
if (!GetComputedStyle())
return;
float hover_delay_seconds = GetComputedStyle()->PopUpShowDelay();
// If the value is infinite or NaN, don't queue a task at all.
DCHECK_GE(hover_delay_seconds, 0);
if (!std::isfinite(hover_delay_seconds))
return;
// It's possible that multiple nested elements have popuphovertarget
// 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* trigger_element, Element* popup_element) {
if (!popup_element ||
!popup_element->HasValidPopupAttribute())
return;
// Remove this element from hoverShowTasks always.
popup_element->GetPopupData()->hoverShowTasks().erase(
trigger_element);
// Only trigger the pop-up if the popuphovertarget attribute
// still points to the same pop-up, and the pop-up is in the
// tree and still not showing.
auto* current_target =
trigger_element->GetTreeScope().getElementById(
trigger_element->FastGetAttribute(
html_names::kPopuphovertargetAttr));
if (popup_element->IsInTreeScope() &&
!popup_element->popupOpen() &&
popup_element == current_target) {
popup_element->InvokePopup(trigger_element);
}
},
WrapWeakPersistent(this), WrapWeakPersistent(popup_element)),
base::Seconds(hover_delay_seconds)));
} else {
// If we have a hover show task still waiting, cancel it. Based on this
// logic, if you hover a popuphovertarget element, then remove the
// popuphovertarget attribute, there will be no way to stop the pop-up from
// being shown after the delay, even if you subsequently de-hover the
// element.
Element* hover_pop_up = PopupHoverTargetElement();
if (!hover_pop_up)
return;
if (auto& hover_tasks = hover_pop_up->GetPopupData()->hoverShowTasks();
hover_tasks.Contains(this)) {
hover_tasks.Take(this).Cancel();
}
}
}

void Element::SetNeedsRepositioningForSelectMenu(bool flag) {
DCHECK(RuntimeEnabledFeatures::HTMLSelectMenuElementEnabled());
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled(
Expand Down Expand Up @@ -8543,7 +8357,6 @@ void Element::SetHovered(bool hovered) {
return;

GetDocument().UserActionElements().SetHovered(this, hovered);
HandlePopupHovered(hovered);

const ComputedStyle* style = GetComputedStyle();
if (!style || style->AffectedByHover()) {
Expand Down
Loading

0 comments on commit 7bf8d3b

Please sign in to comment.