Skip to content

Commit

Permalink
Rename :popup-open pseudo class to :top-layer
Browse files Browse the repository at this point in the history
This CL just does a rename for the :top-layer pseudo class. It does
not change behavior, which is/was that this pseudo class only applies
to elements using the Popup API [1], and not to all elements that
might inhabit the top layer. That will need to wait for a CSSWG
resolution. But for now, the [2] resolution means we should use this
pseudo class for the Popup API prototype starting now.

[1] https://open-ui.org/components/popup.research.explainer
[2] openui/open-ui#470 (comment)

Bug: 1307772
Change-Id: I81a89132b84346a360d7b580f5f39be9da697bdc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3668918
Commit-Queue: Dan Clark <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Dan Clark <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1008430}
NOKEYCHECK=True
GitOrigin-RevId: c3b17215facf56ce6dcd2b660766c73b52df903b
  • Loading branch information
mfreed7 authored and copybara-github committed May 27, 2022
1 parent d6c4459 commit ac72c0b
Show file tree
Hide file tree
Showing 28 changed files with 219 additions and 219 deletions.
8 changes: 4 additions & 4 deletions blink/renderer/core/css/css_selector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,8 @@ PseudoId CSSSelector::GetPseudoId(PseudoType type) {
case kPseudoListBox:
case kPseudoMultiSelectFocus:
case kPseudoHostHasAppearance:
case kPseudoPopupOpen:
case kPseudoSlotted:
case kPseudoTopLayer:
case kPseudoVideoPersistent:
case kPseudoVideoPersistentAncestor:
case kPseudoXrOverlay:
Expand Down Expand Up @@ -452,7 +452,6 @@ const static NameToPseudoStruct kPseudoTypeWithoutArgumentsMap[] = {
{"placeholder", CSSSelector::kPseudoPlaceholder},
{"placeholder-shown", CSSSelector::kPseudoPlaceholderShown},
{"playing", CSSSelector::kPseudoPlaying},
{"popup-open", CSSSelector::kPseudoPopupOpen},
{"read-only", CSSSelector::kPseudoReadOnly},
{"read-write", CSSSelector::kPseudoReadWrite},
{"required", CSSSelector::kPseudoRequired},
Expand All @@ -465,6 +464,7 @@ const static NameToPseudoStruct kPseudoTypeWithoutArgumentsMap[] = {
{"start", CSSSelector::kPseudoStart},
{"target", CSSSelector::kPseudoTarget},
{"target-text", CSSSelector::kPseudoTargetText},
{"top-layer", CSSSelector::kPseudoTopLayer},
{"valid", CSSSelector::kPseudoValid},
{"vertical", CSSSelector::kPseudoVertical},
{"visited", CSSSelector::kPseudoVisited},
Expand Down Expand Up @@ -553,7 +553,7 @@ CSSSelector::PseudoType CSSSelector::NameToPseudoType(const AtomicString& name,
!RuntimeEnabledFeatures::CSSPseudoPlayingPausedEnabled())
return CSSSelector::kPseudoUnknown;

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

Expand Down Expand Up @@ -749,7 +749,6 @@ void CSSSelector::UpdatePseudoType(const AtomicString& value,
case kPseudoPictureInPicture:
case kPseudoPlaceholderShown:
case kPseudoPlaying:
case kPseudoPopupOpen:
case kPseudoReadOnly:
case kPseudoReadWrite:
case kPseudoRelativeLeftmost:
Expand All @@ -761,6 +760,7 @@ void CSSSelector::UpdatePseudoType(const AtomicString& value,
case kPseudoStart:
case kPseudoState:
case kPseudoTarget:
case kPseudoTopLayer:
case kPseudoUnknown:
case kPseudoValid:
case kPseudoVertical:
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/css/css_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ class CORE_EXPORT CSSSelector {
kPseudoListBox,
kPseudoMultiSelectFocus,
kPseudoHostHasAppearance,
kPseudoPopupOpen,
kPseudoTopLayer,
kPseudoSlotted,
kPseudoVideoPersistent,
kPseudoVideoPersistentAncestor,
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/css/parser/css_proto_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ const std::string Converter::kPseudoLookupTable[] = {
"past",
"placeholder",
"placeholder-shown",
"popup-open",
"read-only",
"read-write",
"required",
Expand All @@ -115,6 +114,7 @@ const std::string Converter::kPseudoLookupTable[] = {
"single-button",
"start",
"target",
"top-layer",
"valid",
"vertical",
"visited",
Expand Down
6 changes: 3 additions & 3 deletions blink/renderer/core/css/popup.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@

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

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

Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/core/css/rule_feature_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ bool SupportsInvalidation(CSSSelector::PseudoType type) {
case CSSSelector::kPseudoListBox:
case CSSSelector::kPseudoMultiSelectFocus:
case CSSSelector::kPseudoHostHasAppearance:
case CSSSelector::kPseudoPopupOpen:
case CSSSelector::kPseudoTopLayer:
case CSSSelector::kPseudoSlotted:
case CSSSelector::kPseudoVideoPersistent:
case CSSSelector::kPseudoVideoPersistentAncestor:
Expand Down Expand Up @@ -648,7 +648,7 @@ InvalidationSet* RuleFeatureSet::InvalidationSetForSimpleSelector(
case CSSSelector::kPseudoInRange:
case CSSSelector::kPseudoOutOfRange:
case CSSSelector::kPseudoDefined:
case CSSSelector::kPseudoPopupOpen:
case CSSSelector::kPseudoTopLayer:
case CSSSelector::kPseudoVideoPersistent:
case CSSSelector::kPseudoVideoPersistentAncestor:
case CSSSelector::kPseudoXrOverlay:
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/css/selector_checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,7 @@ bool SelectorChecker::CheckPseudoClass(const SelectorCheckingContext& context,
}
break;
}
case CSSSelector::kPseudoPopupOpen:
case CSSSelector::kPseudoTopLayer:
if (element.HasValidPopupAttribute()) {
return element.popupOpen();
}
Expand Down
4 changes: 2 additions & 2 deletions blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2485,7 +2485,7 @@ void Element::showPopup(ExceptionState& exception_state) {
}
GetPopupData()->setOpen(true);
GetDocument().AddToTopLayer(this);
PseudoStateChanged(CSSSelector::kPseudoPopupOpen);
PseudoStateChanged(CSSSelector::kPseudoTopLayer);
GetPopupData()->setPreviouslyFocusedElement(
should_restore_focus ? GetDocument().FocusedElement() : nullptr);
SetPopupFocusOnShow();
Expand Down Expand Up @@ -2527,7 +2527,7 @@ void Element::hidePopupInternal(HidePopupFocusBehavior focus_behavior) {
GetPopupData()->setInvoker(nullptr);
GetPopupData()->setNeedsRepositioningForSelectMenu(false);
GetDocument().RemoveFromTopLayer(this);
PseudoStateChanged(CSSSelector::kPseudoPopupOpen);
PseudoStateChanged(CSSSelector::kPseudoTopLayer);
// Queue the hide event.
Event* event = Event::CreateBubble(event_type_names::kHide);
event->SetTarget(this);
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/inspector/inspector_trace_events.cc
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ const char* PseudoTypeToString(CSSSelector::PseudoType pseudo_type) {
DEFINE_STRING_MAPPING(PseudoIsHtml)
DEFINE_STRING_MAPPING(PseudoListBox)
DEFINE_STRING_MAPPING(PseudoMultiSelectFocus)
DEFINE_STRING_MAPPING(PseudoPopupOpen)
DEFINE_STRING_MAPPING(PseudoTopLayer)
DEFINE_STRING_MAPPING(PseudoHostHasAppearance)
DEFINE_STRING_MAPPING(PseudoVideoPersistent)
DEFINE_STRING_MAPPING(PseudoVideoPersistentAncestor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,13 @@
const selectMenu1Popup = document.getElementById("selectMenu1-popup");
const selectMenu1Button = document.getElementById("selectMenu1-button");
const selectMenu1Child2 = document.getElementById("selectMenu1-child2");
assert_false(selectMenu1Popup.matches(':popup-open'));
assert_false(selectMenu1Popup.matches(':top-layer'));
selectMenu1Button.click();
assert_false(selectMenu1Popup.matches(':popup-open'), "Clicking a button part that is a descendant of the listbox part should have no effect");
assert_false(selectMenu1Popup.matches(':top-layer'), "Clicking a button part that is a descendant of the listbox part should have no effect");

assert_equals(selectMenu1.value, "one");
await clickOn(selectMenu1);
assert_true(selectMenu1Popup.matches(':popup-open'));
assert_true(selectMenu1Popup.matches(':top-layer'));
await clickOn(selectMenu1Child2);
assert_equals(selectMenu1.value, "two", "Clicking an <option> should change the value");
}, "To receive button part controller code, an element labeled as a button must not be a descendant of the listbox part in a flat tree traversal");
Expand All @@ -226,9 +226,9 @@
const selectMenu2Child2 = document.getElementById("selectMenu2-child2");
const selectMenu2Child4 = document.getElementById("selectMenu2-child4");

assert_false(selectMenu2Popup.matches(':popup-open'));
assert_false(selectMenu2Popup.matches(':top-layer'));
await clickOn(selectMenu2Button);
assert_false(selectMenu2Popup.matches(':popup-open'), "Clicking a button part should not show an invalid listbox part");
assert_false(selectMenu2Popup.matches(':top-layer'), "Clicking a button part should not show an invalid listbox part");

assert_equals(selectMenu2.value, "three");
await clickOn(selectMenu2Button);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@
setup({ explicit_done: true });

popup2.showPopup();
assert_false(popup1.matches(':popup-open'));
assert_true(popup2.matches(':popup-open'));
assert_false(popup1.matches(':top-layer'));
assert_true(popup2.matches(':top-layer'));
await clickOn(button1);
test(t => {
// Button1 is the anchor for popup1, and an ancestor of popup2.
// Since popup2 is open, but not popup1, button1 should not be
// the anchor of any open popup. So popup2 should be closed.
assert_false(popup2.matches(':popup-open'));
assert_true(popup1.matches(':popup-open'));
assert_false(popup2.matches(':top-layer'));
assert_true(popup1.matches(':top-layer'));
},'Nested popups (inside anchor elements) do not affect light dismiss');

done();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
const isVisible = !!(popup.offsetWidth || popup.offsetHeight || popup.getClientRects().length);
if (isVisible) {
assert_not_equals(window.getComputedStyle(popup).display,'none');
assert_equals(isPopup,popup.matches(':popup-open'));
assert_equals(isPopup,popup.matches(':top-layer'));
} else {
assert_equals(window.getComputedStyle(popup).display,'none');
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
}
return isVisible;
}
Expand Down Expand Up @@ -138,17 +138,17 @@
test(() => {
const popup = createPopup();
popup.showPopup();
assert_true(popup.matches(':popup-open'));
assert_true(popup.matches(':top-layer'));
popup.setAttribute('popup','hint'); // Change popup type
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
popup.showPopup();
assert_true(popup.matches(':popup-open'));
assert_true(popup.matches(':top-layer'));
popup.setAttribute('popup','async');
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
popup.showPopup();
assert_true(popup.matches(':popup-open'));
assert_true(popup.matches(':top-layer'));
popup.setAttribute('popup','invalid');
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
},'Changing attribute values should close open popups');


Expand All @@ -157,11 +157,11 @@
const popup = createPopup();
popup.setAttribute('popup',type);
popup.showPopup();
assert_true(popup.matches(':popup-open'));
assert_true(popup.matches(':top-layer'));
popup.remove();
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
document.body.appendChild(popup);
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
},`Removing a visible popup=${type} element from the document should close the popup`);
});
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
requestAnimationFrame(() => {
requestAnimationFrame(() => {
test(function(){
assert_true(p1.matches(':popup-open'),'The first (outermost) popup should be the one that opens in this case');
assert_false(p2.matches(':popup-open'),'The inner popup should not be open');
assert_true(p1.matches(':top-layer'),'The first (outermost) popup should be the one that opens in this case');
assert_false(p2.matches(':top-layer'),'The inner popup should not be open');
}, "The defaultopen attribute should cause only the first popup to open");
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,25 @@
requestAnimationFrame(() => {
requestAnimationFrame(() => {
test(function(){
assert_true(p1.matches(':popup-open'),'defaultopen should open the popup');
assert_true(p1.matches(':top-layer'),'defaultopen should open the popup');
assert_true(p1.hasAttribute('defaultopen'));
assert_true(p1.defaultOpen,'defaultopen should be reflected in the IDL attribute');
assert_false(p2.matches(':popup-open'), 'Only the first popup with defaultopen should be open on load');
assert_false(p2.matches(':top-layer'), 'Only the first popup with defaultopen should be open on load');
assert_true(p2.hasAttribute('defaultopen'),'defaultopen should be present/true, even if not opened');
assert_true(p2.defaultOpen,'defaultopen should be present/true, even if not opened');

assert_false(p2b.matches(':popup-open'),'Only the first popup/hint with defaultopen should be open on load');
assert_true(p4.matches(':popup-open'),'defaultopen should open all async popups');
assert_true(p5.matches(':popup-open'),'defaultopen should open all async popups');
assert_false(p2b.matches(':top-layer'),'Only the first popup/hint with defaultopen should be open on load');
assert_true(p4.matches(':top-layer'),'defaultopen should open all async popups');
assert_true(p5.matches(':top-layer'),'defaultopen should open all async popups');

assert_false(p3.matches(':popup-open'));
assert_false(p3.matches(':top-layer'));
p3.setAttribute('defaultopen','');
assert_false(p3.matches(':popup-open'), 'Changing defaultopen should not affect open status');
assert_false(p3.matches(':top-layer'), 'Changing defaultopen should not affect open status');
assert_true(p3.hasAttribute('defaultopen'));
assert_true(p3.defaultOpen,'defaultopen should still reflect to IDL');

p1.removeAttribute('defaultopen');
assert_true(p1.matches(':popup-open'),'removing defaultopen should not close the popup');
assert_true(p1.matches(':top-layer'),'removing defaultopen should not close the popup');
assert_false(p1.hasAttribute('defaultopen'),'...but it should reflect to IDL');

p1.hidePopup();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,26 @@
let showCount = 0;
let hideCount = 0;
await new Promise(resolve => window.addEventListener('load',() => resolve()));
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
document.addEventListener('show',() => ++showCount);
document.addEventListener('hide',() => ++hideCount);
assert_equals(0,showCount);
assert_equals(0,hideCount);
popup.showPopup();
assert_true(popup.matches(':popup-open'));
assert_true(popup.matches(':top-layer'));
await waitUntilChange(() => showCount);
assert_equals(1,showCount);
assert_equals(0,hideCount);
await requestAnimationFramePromise();
assert_true(popup.matches(':popup-open'));
assert_true(popup.matches(':top-layer'));
popup.hidePopup();
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
await waitUntilChange(() => hideCount);
assert_equals(1,showCount);
assert_equals(1,hideCount);
await requestAnimationFramePromise();
// No additional events after animation frame
assert_false(popup.matches(':popup-open'));
assert_false(popup.matches(':top-layer'));
assert_equals(1,showCount);
assert_equals(1,hideCount);
}, 'Show and hide events get properly dispatched for popups');
Expand Down
Loading

0 comments on commit ac72c0b

Please sign in to comment.