Skip to content

Commit

Permalink
Allow hide animations to be started from pop up hide event
Browse files Browse the repository at this point in the history 
See [1] for the origin of this change, which makes it possible to
trigger pop up hide animations from within the `hide` event handler.
For example:

  popup.addEventListener('hide', () => {
    popup.animate({
      transform: 'translateY(-50px)',
      opacity: 0,
    }, 200);
  });

To accomplish that, the hide process now looks like this:

 1. Capture any already-running animations via getAnimations(),
    including animations on descendent elements.
 2. Remove the :top-layer pseudo class.
 3. Fire the 'hide' event.
 4. If the hidePopup() call is *not* the result of the pop-up being
    "forced out" of the top layer, e.g. by a modal dialog or fullscreen
    element:
   a. Restore focus to the previously-focused element.
   b. Update style. (Animations/transitions start here.)
   c. Call getAnimations() again, remove any from step chromium#1, and then wait
      until all of them finish or are cancelled.
 5. Remove the pop-up from the top layer, and add the UA display:none
    style.
 6. Update style.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3688871/9/third_party/blink/renderer/core/dom/element.cc#2660

Bug: 1307772

Change-Id: I910535b13cfc3c8f8498ed64dae73caa75dd7317
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708419
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1018685}
  • Loading branch information
Mason Freed authored and Chromium LUCI CQ committed Jun 28, 2022
1 parent 28b51cf commit c58a6b9
Show file tree
Hide file tree
Showing 10 changed files with 216 additions and 117 deletions.
179 changes: 105 additions & 74 deletions third_party/blink/renderer/core/dom/element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2448,7 +2448,7 @@ void Element::UpdatePopupAttribute(String value) {
return;
// If the popup type is changing, hide it.
if (popupOpen()) {
hidePopUpInternal(HidePopupFocusBehavior::kFocusPreviousElement,
HidePopUpInternal(HidePopupFocusBehavior::kFocusPreviousElement,
HidePopupForcingLevel::kHideAfterAnimations);
}
}
Expand Down Expand Up @@ -2490,6 +2490,14 @@ bool Element::popupOpen() const {
return false;
}

// Showing a pop-up happens in phases, to facilitate animations and
// transitions:
// 1. Move the pop-up to the top layer, and remove the UA display:none
// style.
// 2. Update style. (Transition initial style can be specified in this
// state.)
// 3. Set the :top-layer pseudo class.
// 4. Update style. (Animations/transitions happen here.)
void Element::showPopUp(ExceptionState& exception_state) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
if (!HasValidPopupAttribute()) {
Expand All @@ -2502,11 +2510,12 @@ void Element::showPopUp(ExceptionState& exception_state) {
DOMExceptionCode::kInvalidStateError,
"Invalid on already-showing or disconnected popup elements");
}

bool should_restore_focus = false;
if (PopupType() == PopupValueType::kAuto ||
PopupType() == PopupValueType::kHint) {
if (GetDocument().HintShowing()) {
GetDocument().PopupAndHintStack().back()->hidePopUpInternal(
GetDocument().PopupAndHintStack().back()->HidePopUpInternal(
HidePopupFocusBehavior::kNone,
HidePopupForcingLevel::kHideAfterAnimations);
}
Expand All @@ -2524,6 +2533,8 @@ void Element::showPopUp(ExceptionState& exception_state) {
should_restore_focus = stack.IsEmpty();
stack.push_back(this);
}

GetPopupData()->setAnimationFinishedListener(nullptr);
GetPopupData()->setPreviouslyFocusedElement(
should_restore_focus ? GetDocument().FocusedElement() : nullptr);
GetDocument().AddToTopLayer(this);
Expand All @@ -2542,10 +2553,11 @@ void Element::showPopUp(ExceptionState& exception_state) {
PseudoStateChanged(CSSSelector::kPseudoTopLayer);

SetPopupFocusOnShow();
// Queue the show event.
// Fire the show event (bubbles, not cancelable).
Event* event = Event::CreateBubble(event_type_names::kShow);
event->SetTarget(this);
GetDocument().EnqueueAnimationFrameEvent(event);
auto result = DispatchEvent(*event);
DCHECK_EQ(result, DispatchEventResult::kNotCanceled);
}

// static
Expand All @@ -2559,11 +2571,11 @@ void Element::HideAllPopupsUntil(const Element* endpoint,
if (forcing_level == HidePopupForcingLevel::kHideImmediately) {
auto popups_to_hide = document.PopupsWaitingToHide();
for (auto popup : popups_to_hide)
popup->FinishPopupHideIfNeeded(forcing_level);
popup->PopupHideFinishIfNeeded();
}
while (!document.PopupAndHintStack().IsEmpty() &&
document.PopupAndHintStack().back() != endpoint) {
document.PopupAndHintStack().back()->hidePopUpInternal(focus_behavior,
document.PopupAndHintStack().back()->HidePopUpInternal(focus_behavior,
forcing_level);
}
}
Expand All @@ -2580,32 +2592,51 @@ void Element::hidePopUp(ExceptionState& exception_state) {
DOMExceptionCode::kInvalidStateError,
"Invalid on already-hidden popup elements");
}
hidePopUpInternal(HidePopupFocusBehavior::kFocusPreviousElement,
HidePopUpInternal(HidePopupFocusBehavior::kFocusPreviousElement,
HidePopupForcingLevel::kHideAfterAnimations);
}

void Element::hidePopUpInternal(HidePopupFocusBehavior focus_behavior,
// Hiding a pop-up happens in phases, to facilitate animations and
// transitions:
// 1. Capture any already-running animations via getAnimations(), including
// animations on descendent elements.
// 2. Remove the :top-layer pseudo class.
// 3. Fire the 'hide' event.
// 4. If the hidePopup() call is *not* the result of the pop-up being "forced
// out" of the top layer, e.g. by a modal dialog or fullscreen element:
// a. Restore focus to the previously-focused element.
// b. Update style. (Animations/transitions start here.)
// c. Call getAnimations() again, remove any from step #1, and then wait
// until all of them finish or are cancelled.
// 5. Remove the pop-up from the top layer, and add the UA display:none style.
// 6. Update style.
void Element::HidePopUpInternal(HidePopupFocusBehavior focus_behavior,
HidePopupForcingLevel forcing_level) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
DCHECK(HasValidPopupAttribute());
if (!isConnected() || !popupOpen())
return;
if (PopupType() == PopupValueType::kAuto ||
PopupType() == PopupValueType::kHint) {
// Hide any popups/hints above us in the stack.
HideAllPopupsUntil(this, GetDocument(), focus_behavior, forcing_level);
// Then remove this popup/hint from the stack.
// Then remove this popup/hint from the stack, if present. If the popup
// is already hidden, it won't be in the stack.
auto& stack = GetDocument().PopupAndHintStack();
DCHECK(stack.back() == this);
stack.pop_back();
if (!stack.IsEmpty() && stack.back() == this)
stack.pop_back();
DCHECK(stack.IsEmpty() || !stack.Contains(this));
}
if (!isConnected() || !popupOpen()) {
DCHECK(!GetDocument().PopupAndHintStack().Contains(this));
return;
}
GetDocument().PopupsWaitingToHide().insert(this);

// Grab already-running animations first, before removing :top-layer, so we
// know to ignore those. Make sure to get animations on descendant elements.
bool force_hide = forcing_level == HidePopupForcingLevel::kHideImmediately;
HeapVector<Member<Animation>> previous_animations;
auto animation_options = GetAnimationsOptionsResolved{.use_subtree = true};
if (forcing_level != HidePopupForcingLevel::kHideImmediately)
previous_animations = GetAnimationsInternal(animation_options);
if (!force_hide) {
previous_animations = GetAnimationsInternal(
GetAnimationsOptionsResolved{.use_subtree = true});
}

GetPopupData()->setInvoker(nullptr);
GetPopupData()->setNeedsRepositioningForSelectMenu(false);
Expand All @@ -2614,68 +2645,68 @@ void Element::hidePopUpInternal(HidePopupFocusBehavior focus_behavior,
GetPopupData()->setVisibilityState(PopupVisibilityState::kTransitioning);
PseudoStateChanged(CSSSelector::kPseudoTopLayer);

if (forcing_level == HidePopupForcingLevel::kHideImmediately ||
!HasAnimations()) {
// If we don't have hide animations, or we need to hide immediately, finish
// the hiding process now. Otherwise, this will get finished when animations
// complete.
FinishPopupHideIfNeeded(forcing_level);
// Fire the hide event (bubbles, not cancelable).
Event* event = Event::CreateBubble(event_type_names::kHide);
event->SetTarget(this);
if (force_hide) {
// We will be force-hidden when the pop-up element is being removed from
// the document, during which event dispatch is prohibited.
GetDocument().EnqueueAnimationFrameEvent(event);
// Immediately finish the hide process.
return PopupHideFinishIfNeeded();
}
auto result = DispatchEvent(*event);
DCHECK_EQ(result, DispatchEventResult::kNotCanceled);

// The 'hide' event handler could have changed this popup, e.g. by changing
// its type, removing it from the document, or calling showPopUp().
if (!isConnected() || !HasValidPopupAttribute() ||
GetPopupData()->visibilityState() !=
PopupVisibilityState::kTransitioning) {
return;
}

// Grab all animations, so that we can "finish" the hide operation once
// they complete. This will *also* force a style update, ensuring property
// values are set after `:top-layer` stops matching, so that transitions
// can start.
HeapHashSet<Member<EventTarget>> animations;
for (const auto& animation : GetAnimationsInternal(
GetAnimationsOptionsResolved{.use_subtree = true})) {
animations.insert(animation);
}
animations.RemoveAll(previous_animations);
if (animations.IsEmpty()) {
// No animations to wait for: just finish immediately.
PopupHideFinishIfNeeded();
} else {
// Grab all animations, so that we can "finish" the hide operation once
// they complete. This will *also* force a style update, ensuring property
// values are set after `:top-layer` stops matching, so that transitions
// can start.
HeapHashSet<Member<EventTarget>> animations;
for (const auto& animation : GetAnimationsInternal(animation_options)) {
animations.insert(animation);
}
animations.RemoveAll(previous_animations);
if (animations.IsEmpty()) {
// All animations were pre-existing. Just finish now.
FinishPopupHideIfNeeded(forcing_level);
} else {
GetPopupData()->setAnimationFinishedListener(
MakeGarbageCollected<PopupAnimationFinishedEventListener>(
this, std::move(animations)));
GetPopupData()->setAnimationFinishedListener(
MakeGarbageCollected<PopupAnimationFinishedEventListener>(
this, std::move(animations)));
}

Element* previously_focused_element =
GetPopupData()->previouslyFocusedElement();
if (previously_focused_element) {
GetPopupData()->setPreviouslyFocusedElement(nullptr);
if (GetPopupData()->focusBehavior() ==
HidePopupFocusBehavior::kFocusPreviousElement) {
FocusOptions* focus_options = FocusOptions::Create();
focus_options->setPreventScroll(true);
previously_focused_element->Focus(focus_options);
}
}
}

void Element::FinishPopupHideIfNeeded(HidePopupForcingLevel forcing_level) {
if (!GetPopupData() || GetPopupData()->visibilityState() !=
PopupVisibilityState::kTransitioning) {
return;
}
DCHECK(GetDocument().PopupsWaitingToHide().Contains(this));
void Element::PopupHideFinishIfNeeded() {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
GetDocument().PopupsWaitingToHide().erase(this);
GetDocument().RemoveFromTopLayer(this);
// Re-apply display:none.
GetPopupData()->setVisibilityState(PopupVisibilityState::kHidden);
GetPopupData()->setAnimationFinishedListener(nullptr);
PseudoStateChanged(CSSSelector::kPseudoPopupHidden);
if (forcing_level == HidePopupForcingLevel::kHideImmediately) {
// If we're hiding immediately, we do not fire the "hide" event, and we do
// not focus the previously-focused element. Either of these could cause
// more popups to be shown.
GetPopupData()->setPreviouslyFocusedElement(nullptr);
} else {
// Queue the hide event.
Event* event = Event::CreateBubble(event_type_names::kHide);
event->SetTarget(this);
GetDocument().EnqueueAnimationFrameEvent(event);
if (Element* previously_focused_element =
GetPopupData()->previouslyFocusedElement()) {
GetPopupData()->setPreviouslyFocusedElement(nullptr);
if (GetPopupData()->focusBehavior() ==
HidePopupFocusBehavior::kFocusPreviousElement) {
FocusOptions* focus_options = FocusOptions::Create();
focus_options->setPreventScroll(true);
// Call Focus() last, since it will fire a focus event which could
// modify this element. Focusing this element may also hide other
// popups.
previously_focused_element->Focus(focus_options);
}
}
if (GetPopupData()) {
GetPopupData()->setVisibilityState(PopupVisibilityState::kHidden);
GetPopupData()->setAnimationFinishedListener(nullptr);
PseudoStateChanged(CSSSelector::kPseudoPopupHidden);
}
}

Expand Down Expand Up @@ -2872,7 +2903,7 @@ void Element::HandlePopupLightDismiss(const Event& event) {
const KeyboardEvent* key_event = DynamicTo<KeyboardEvent>(event);
if (key_event && key_event->key() == "Escape") {
// Escape key just pops the topmost popup or hint off the stack.
document.PopupAndHintStack().back()->hidePopUpInternal(
document.PopupAndHintStack().back()->HidePopUpInternal(
HidePopupFocusBehavior::kFocusPreviousElement,
HidePopupForcingLevel::kHideAfterAnimations);
}
Expand Down Expand Up @@ -3286,7 +3317,7 @@ void Element::RemovedFrom(ContainerNode& insertion_point) {
// removed from the popup element stack and the top layer.
if (was_in_document && HasValidPopupAttribute()) {
// We can't run focus event handlers while removing elements.
hidePopUpInternal(HidePopupFocusBehavior::kNone,
HidePopUpInternal(HidePopupFocusBehavior::kNone,
HidePopupForcingLevel::kHideImmediately);
}

Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/dom/element.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,9 +571,9 @@ class CORE_EXPORT Element : public ContainerNode, public Animatable {
bool popupOpen() const;
void showPopUp(ExceptionState& exception_state);
void hidePopUp(ExceptionState& exception_state);
void hidePopUpInternal(HidePopupFocusBehavior focus_behavior,
void HidePopUpInternal(HidePopupFocusBehavior focus_behavior,
HidePopupForcingLevel forcing_level);
void FinishPopupHideIfNeeded(HidePopupForcingLevel);
void PopupHideFinishIfNeeded();
static const Element* NearestOpenAncestralPopup(Node* start_node);
static void HandlePopupLightDismiss(const Event& event);
void InvokePopup(Element* invoker);
Expand Down
5 changes: 3 additions & 2 deletions third_party/blink/renderer/core/dom/popup_animation_finished_event_listener.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ PopupAnimationFinishedEventListener::PopupAnimationFinishedEventListener(
Member<Element> popup_element,
HeapHashSet<Member<EventTarget>>&& animations)
: popup_element_(popup_element), animations_(std::move(animations)) {
DCHECK(popup_element->HasValidPopupAttribute());
DCHECK(!animations_.IsEmpty());
for (auto animation : animations_) {
animation->addEventListener(event_type_names::kFinish, this,
/*use_capture*/ false);
Expand Down Expand Up @@ -58,8 +60,7 @@ void PopupAnimationFinishedEventListener::Invoke(ExecutionContext*,

// Finish hiding the popup once all animations complete.
if (animations_.IsEmpty()) {
popup_element_->FinishPopupHideIfNeeded(
HidePopupForcingLevel::kHideAfterAnimations);
popup_element_->PopupHideFinishIfNeeded();
}
}

Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/forms/html_form_control_element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ void HTMLFormControlElement::DefaultEventHandler(Event& event) {
event.type() == event_type_names::kDOMActivate &&
(!Form() || !IsSuccessfulSubmitButton())) {
if (can_hide) {
popup.element->hidePopUpInternal(
popup.element->HidePopUpInternal(
HidePopupFocusBehavior::kFocusPreviousElement,
HidePopupForcingLevel::kHideAfterAnimations);
} else if (can_show) {
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/html/forms/html_select_menu_element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ void HTMLSelectMenuElement::CloseListbox() {
if (listbox_part_ && open()) {
if (listbox_part_->HasValidPopupAttribute()) {
// We will handle focus directly.
listbox_part_->hidePopUpInternal(
listbox_part_->HidePopUpInternal(
HidePopupFocusBehavior::kNone,
HidePopupForcingLevel::kHideAfterAnimations);
}
Expand Down
51 changes: 50 additions & 1 deletion .../web_tests/external/wpt/html/semantics/popups/popup-animation-corner-cases.tentative.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
assert_equals(popUp.getAnimations({subtree: true}).length,0);
// Start an animation on the popUp and its descendent.
popUp.animate([{opacity: 1},{opacity: 0}],{duration: 1000000,iterations: 1});
descendent.animate([{transform: 'rotate(0)'},{transform: 'rotate(360deg)'}],{duration: 1000000,iterations: 1});
descendent.animate([{transform: 'rotate(0)'},{transform: 'rotate(360deg)'}],1000000);
assert_equals(popUp.getAnimations({subtree: true}).length,2);
// Then hide the popUp.
popUp.hidePopUp();
Expand All @@ -87,4 +87,53 @@
assert_equals(popUp.getAnimations({subtree: true}).length,2,'animations should still be running');
assert_false(isElementVisible(popUp),'Pre-existing animations should not keep the pop up visible');
},'Pre-existing animations should *not* keep the pop up visible until the animation ends');

promise_test(async (t) => {
const {popUp, descendent} = createPopUp(t,'');
popUp.showPopUp();
assert_true(isElementVisible(popUp));
assert_equals(popUp.getAnimations({subtree: true}).length,0);
let animation;
popUp.addEventListener('hide', () => {
animation = popUp.animate([{opacity: 1},{opacity: 0}],1000000);
});
assert_equals(popUp.getAnimations({subtree: true}).length,0,'There should be no animations yet');
popUp.hidePopUp();
assert_equals(popUp.getAnimations({subtree: true}).length,1,'the hide animation should now be running');
assert_true(!!animation);
assert_true(isElementVisible(popUp),'The animation should keep the popup visible');
animation.finish();
await waitForRender();
assert_false(isElementVisible(popUp),'Once the animation ends, the popup is hidden');
},'It should be possible to use the "hide" event handler to animate the hide');


promise_test(async (t) => {
const {popUp, descendent} = createPopUp(t,'');
const dialog = document.body.appendChild(document.createElement('dialog'));
t.add_cleanup(() => dialog.remove());
popUp.showPopUp();
assert_true(isElementVisible(popUp));
assert_equals(popUp.getAnimations({subtree: true}).length,0);
popUp.addEventListener('hide', () => {
popUp.animate([{opacity: 1},{opacity: 0}],1000000);
});
assert_equals(popUp.getAnimations({subtree: true}).length,0,'There should be no animations yet');
dialog.showModal(); // Force hide the popup
await waitForRender();
assert_equals(popUp.getAnimations({subtree: true}).length,1,'the hide animation should now be running');
assert_false(isElementVisible(popUp),'But the animation should *not* keep the popup visible in this case');
},'It should *not* be possible to use the "hide" event handler to animate the hide, if the hide is due to dialog.showModal');

promise_test(async (t) => {
const {popUp, descendent} = createPopUp(t,'');
popUp.showPopUp();
assert_true(isElementVisible(popUp));
popUp.addEventListener('hide', (e) => {
e.preventDefault();
});
popUp.hidePopUp();
await waitForRender();
assert_false(isElementVisible(popUp),'Even if hide event is cancelled, the popup still closes');
},'hide event cannot be cancelled');
</script>
Loading

0 comments on commit c58a6b9

Please sign in to comment.