Skip to content

Commit

Permalink
Make popups animation-friendly
Browse files Browse the repository at this point in the history
Now, popups will follow this process when showing/hiding:

showPopup():
 1. Move the popup 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.)

hidePopup():
 1. Capture any already-running animations via getAnimations().
 2. Remove the :top-layer pseudo class.
 3. Update style. (Animations/transitions start here.)
 4. If the hidePopup() call is not due to a "force out" situation,
    getAnimations() again, remove any from step #1, and then wait here
    until all of them finish or are cancelled.
 4. Remove the popup from the top layer, and add the UA display:none
    style.
 5. Update style.

See this issue for more details:
  openui/open-ui#335

Bug: 1307772
Change-Id: Ia20eb6e9533c1a0b1029ca1279d42fe2648300af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688871
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1014235}
NOKEYCHECK=True
GitOrigin-RevId: f659aa9aee9f710b49e821fff05896eba5aa0a5c
  • Loading branch information
mfreed7 authored and copybara-github committed Jun 15, 2022
1 parent 8a2598c commit fef14b6
Show file tree
Hide file tree
Showing 28 changed files with 722 additions and 119 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ message Selector {
_INTERNAL_LIST_BOX = 4;
_INTERNAL_MEDIA_CONTROLS_OVERLAY_CAST_BUTTON = 5;
_INTERNAL_MULTI_SELECT_FOCUS = 6;
_INTERNAL_POPUP_OPEN = 7;
_INTERNAL_POPUP_HIDDEN = 7;
_INTERNAL_SHADOW_HOST_HAS_APPEARANCE = 8;
_INTERNAL_SPATIAL_NAVIGATION_FOCUS = 9;
_INTERNAL_VIDEO_PERSISTENT = 10;
Expand Down Expand Up @@ -1023,21 +1023,22 @@ message Selector {
SINGLE_BUTTON = 76;
START = 77;
TARGET = 78;
VALID = 79;
VERTICAL = 80;
VISITED = 81;
WINDOW_INACTIVE = 82;
_WEBKIT_ANY = 83;
HOST_CONTEXT = 84;
LANG = 85;
NOT = 86;
NTH_CHILD = 87;
NTH_LAST_CHILD = 88;
NTH_LAST_OF_TYPE = 89;
NTH_OF_TYPE = 90;
SLOTTED = 91;
XR_OVERLAY = 92;
INVALID_PSEUDO_VALUE = 93;
TOP_LAYER = 79;
VALID = 80;
VERTICAL = 81;
VISITED = 82;
WINDOW_INACTIVE = 83;
_WEBKIT_ANY = 84;
HOST_CONTEXT = 85;
LANG = 86;
NOT = 87;
NTH_CHILD = 88;
NTH_LAST_CHILD = 89;
NTH_LAST_OF_TYPE = 90;
NTH_OF_TYPE = 91;
SLOTTED = 92;
XR_OVERLAY = 93;
INVALID_PSEUDO_VALUE = 94;
}
optional PseudoValueId pseudo_value_id = 4;
required Combinator combinator = 5;
Expand Down
13 changes: 10 additions & 3 deletions blink/renderer/core/animation/animatable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,20 @@ Animation* Animatable::animate(ScriptState* script_state,
HeapVector<Member<Animation>> Animatable::getAnimations(
GetAnimationsOptions* options) {
bool use_subtree = options && options->subtree();
return GetAnimationsInternal(
GetAnimationsOptionsResolved{.use_subtree = use_subtree});
}

HeapVector<Member<Animation>> Animatable::GetAnimationsInternal(
GetAnimationsOptionsResolved options) {
Element* element = GetAnimationTarget();
if (use_subtree)
if (options.use_subtree)
element->GetDocument().UpdateStyleAndLayoutTreeForSubtree(element);
else
element->GetDocument().UpdateStyleAndLayoutTreeForNode(element);

HeapVector<Member<Animation>> animations;
if (!use_subtree && !element->HasAnimations())
if (!options.use_subtree && !element->HasAnimations())
return animations;

for (const auto& animation :
Expand All @@ -154,7 +160,8 @@ HeapVector<Member<Animation>> Animatable::getAnimations(
DCHECK(animation->effect());
// TODO(gtsteel) make this use the idl properties
Element* target = To<KeyframeEffect>(animation->effect())->EffectTarget();
if (element == target || (use_subtree && element->contains(target))) {
if (element == target ||
(options.use_subtree && element->contains(target))) {
// DocumentAnimations::getAnimations should only give us animations that
// are either current or in effect.
DCHECK(animation->effect()->IsCurrent() ||
Expand Down
7 changes: 7 additions & 0 deletions blink/renderer/core/animation/animatable.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ class ScriptState;
class ScriptValue;
class V8UnionKeyframeAnimationOptionsOrUnrestrictedDouble;

struct GetAnimationsOptionsResolved {
bool use_subtree;
};

// https://drafts.csswg.org/web-animations-1/#the-animatable-interface-mixin
class CORE_EXPORT Animatable {
public:
Expand All @@ -62,6 +66,9 @@ class CORE_EXPORT Animatable {

HeapVector<Member<Animation>> getAnimations(
GetAnimationsOptions* options = nullptr);

HeapVector<Member<Animation>> GetAnimationsInternal(
GetAnimationsOptionsResolved options);
};

} // namespace blink
Expand Down
7 changes: 7 additions & 0 deletions blink/renderer/core/css/css_selector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ PseudoId CSSSelector::GetPseudoId(PseudoType type) {
case kPseudoHostHasAppearance:
case kPseudoSlotted:
case kPseudoTopLayer:
case kPseudoPopupHidden:
case kPseudoVideoPersistent:
case kPseudoVideoPersistentAncestor:
case kPseudoXrOverlay:
Expand Down Expand Up @@ -374,6 +375,7 @@ const static NameToPseudoStruct kPseudoTypeWithoutArgumentsMap[] = {
CSSSelector::kPseudoWebKitCustomElement},
{"-internal-modal", CSSSelector::kPseudoModal},
{"-internal-multi-select-focus", CSSSelector::kPseudoMultiSelectFocus},
{"-internal-popup-hidden", CSSSelector::kPseudoPopupHidden},
{"-internal-relative-anchor", CSSSelector::kPseudoRelativeAnchor},
{"-internal-selector-fragment-anchor",
CSSSelector::kPseudoSelectorFragmentAnchor},
Expand Down Expand Up @@ -557,6 +559,10 @@ CSSSelector::PseudoType CSSSelector::NameToPseudoType(const AtomicString& name,
!RuntimeEnabledFeatures::HTMLPopupAttributeEnabled())
return CSSSelector::kPseudoUnknown;

if (match->type == CSSSelector::kPseudoPopupHidden &&
!RuntimeEnabledFeatures::HTMLPopupAttributeEnabled())
return CSSSelector::kPseudoUnknown;

if (match->type == CSSSelector::kPseudoHighlight &&
!RuntimeEnabledFeatures::HighlightAPIEnabled()) {
return CSSSelector::kPseudoUnknown;
Expand Down Expand Up @@ -749,6 +755,7 @@ void CSSSelector::UpdatePseudoType(const AtomicString& value,
case kPseudoPictureInPicture:
case kPseudoPlaceholderShown:
case kPseudoPlaying:
case kPseudoPopupHidden:
case kPseudoReadOnly:
case kPseudoReadWrite:
case kPseudoRelativeAnchor:
Expand Down
1 change: 1 addition & 0 deletions blink/renderer/core/css/css_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ class CORE_EXPORT CSSSelector {
kPseudoMultiSelectFocus,
kPseudoHostHasAppearance,
kPseudoTopLayer,
kPseudoPopupHidden,
kPseudoSlotted,
kPseudoVideoPersistent,
kPseudoVideoPersistentAncestor,
Expand Down
1 change: 1 addition & 0 deletions blink/renderer/core/css/parser/css_proto_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const std::string Converter::kPseudoLookupTable[] = {
"-internal-list-box",
"-internal-media-controls-overlay-cast-button",
"-internal-multi-select-focus",
"-internal-popup-hidden",
"-internal-shadow-host-has-appearance",
"-internal-spatial-navigation-focus",
"-internal-video-persistent",
Expand Down
5 changes: 1 addition & 4 deletions blink/renderer/core/css/popup.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@

@namespace "http://www.w3.org/1999/xhtml";

[popup="" i]:not(:top-layer),
[popup=auto i]:not(:top-layer),
[popup=hint i]:not(:top-layer),
[popup=async i]:not(:top-layer) {
[popup]:-internal-popup-hidden {
display: none;
}

Expand Down
2 changes: 2 additions & 0 deletions blink/renderer/core/css/rule_feature_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ bool SupportsInvalidation(CSSSelector::PseudoType type) {
case CSSSelector::kPseudoMultiSelectFocus:
case CSSSelector::kPseudoHostHasAppearance:
case CSSSelector::kPseudoTopLayer:
case CSSSelector::kPseudoPopupHidden:
case CSSSelector::kPseudoSlotted:
case CSSSelector::kPseudoVideoPersistent:
case CSSSelector::kPseudoVideoPersistentAncestor:
Expand Down Expand Up @@ -651,6 +652,7 @@ InvalidationSet* RuleFeatureSet::InvalidationSetForSimpleSelector(
case CSSSelector::kPseudoOutOfRange:
case CSSSelector::kPseudoDefined:
case CSSSelector::kPseudoTopLayer:
case CSSSelector::kPseudoPopupHidden:
case CSSSelector::kPseudoVideoPersistent:
case CSSSelector::kPseudoVideoPersistentAncestor:
case CSSSelector::kPseudoXrOverlay:
Expand Down
7 changes: 7 additions & 0 deletions blink/renderer/core/css/selector_checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "third_party/blink/renderer/core/dom/flat_tree_traversal.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/dom/nth_index_cache.h"
#include "third_party/blink/renderer/core/dom/popup_data.h"
#include "third_party/blink/renderer/core/dom/shadow_root.h"
#include "third_party/blink/renderer/core/dom/text.h"
#include "third_party/blink/renderer/core/editing/frame_selection.h"
Expand Down Expand Up @@ -1394,6 +1395,12 @@ bool SelectorChecker::CheckPseudoClass(const SelectorCheckingContext& context,
return element.popupOpen();
}
return false;
case CSSSelector::kPseudoPopupHidden:
if (element.HasValidPopupAttribute()) {
return element.GetPopupData()->visibilityState() ==
PopupVisibilityState::kHidden;
}
return false;
case CSSSelector::kPseudoFullscreen:
// fall through
case CSSSelector::kPseudoFullScreen:
Expand Down
2 changes: 2 additions & 0 deletions blink/renderer/core/dom/build.gni
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ blink_core_sources_dom = [
"nth_index_cache.h",
"parent_node.h",
"parser_content_policy.h",
"popup_animation_finished_event_listener.cc",
"popup_animation_finished_event_listener.h",
"popup_data.h",
"presentation_attribute_style.cc",
"presentation_attribute_style.h",
Expand Down
33 changes: 1 addition & 32 deletions blink/renderer/core/dom/document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7225,38 +7225,6 @@ bool Document::HintShowing() const {
(popup_and_hint_stack_.back()->PopupType() == PopupValueType::kHint);
}

void Document::HideTopmostPopupOrHint(HidePopupFocusBehavior focus_behavior) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
if (popup_and_hint_stack_.IsEmpty())
return;
popup_and_hint_stack_.back()->hidePopupInternal(focus_behavior);
}

void Document::HideAllPopupsUntil(const Element* endpoint,
HidePopupFocusBehavior focus_behavior) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
while (!popup_and_hint_stack_.IsEmpty() &&
popup_and_hint_stack_.back() != endpoint) {
popup_and_hint_stack_.back()->hidePopupInternal(focus_behavior);
}
}

void Document::HidePopupIfShowing(Element* popup,
HidePopupFocusBehavior focus_behavior) {
DCHECK(RuntimeEnabledFeatures::HTMLPopupAttributeEnabled());
DCHECK(popup->HasValidPopupAttribute());
if (!popup->popupOpen())
return;
if (popup->PopupType() == PopupValueType::kAsync) {
popup->hidePopupInternal(focus_behavior);
} else {
HideAllPopupsUntil(popup, focus_behavior);
DCHECK(!popup_and_hint_stack_.IsEmpty() &&
popup_and_hint_stack_.back() == popup);
HideTopmostPopupOrHint(focus_behavior);
}
}

void Document::exitPointerLock() {
if (!GetPage())
return;
Expand Down Expand Up @@ -8009,6 +7977,7 @@ void Document::Trace(Visitor* visitor) const {
visitor->Trace(node_lists_);
visitor->Trace(top_layer_elements_);
visitor->Trace(popup_and_hint_stack_);
visitor->Trace(popups_waiting_to_hide_);
visitor->Trace(load_event_delay_timer_);
visitor->Trace(plugin_loading_timer_);
visitor->Trace(elem_sheet_);
Expand Down
17 changes: 7 additions & 10 deletions blink/renderer/core/dom/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ class VisitedLinkState;
class WebMouseEvent;
class WorkletAnimationController;
enum class CSSPropertyID;
enum class HidePopupFocusBehavior;
struct AnnotatedRegionValue;
struct FocusParams;
struct IconURL;
Expand Down Expand Up @@ -1511,17 +1510,11 @@ class CORE_EXPORT Document : public ContainerNode,
HeapVector<Member<Element>>& PopupAndHintStack() {
return popup_and_hint_stack_;
}
HeapHashSet<Member<Element>>& PopupsWaitingToHide() {
return popups_waiting_to_hide_;
}
bool PopupOrHintShowing() const;
bool HintShowing() const;
void HideTopmostPopupOrHint(HidePopupFocusBehavior focus_behavior);
// This hides all visible popups up to, but not including,
// |endpoint|. If |endpoint| is nullptr, all popups are hidden.
void HideAllPopupsUntil(const Element* endpoint,
HidePopupFocusBehavior focus_behavior);
// This hides the provided popup, if it is showing. This will also
// hide all popups above |popup| in the popup stack.
void HidePopupIfShowing(Element* popup,
HidePopupFocusBehavior focus_behavior);

// A non-null template_document_host_ implies that |this| was created by
// EnsureTemplateDocument().
Expand Down Expand Up @@ -2321,6 +2314,10 @@ class CORE_EXPORT Document : public ContainerNode,
// hint in the stack, it is at the top.
HeapVector<Member<Element>> popup_and_hint_stack_;

// A set of popups for which hidePopup() has been called, but animations are
// still running.
HeapHashSet<Member<Element>> popups_waiting_to_hide_;

int load_event_delay_count_;

// Objects and embeds depend on "being rendered" for delaying the load event.
Expand Down
Loading

0 comments on commit fef14b6

Please sign in to comment.