Skip to content

Commit

Permalink
Remove :closed pseudo-class
Browse files Browse the repository at this point in the history
This was resolved here:
w3c/csswg-drafts#11039 (comment)

Change-Id: Icfe2bbf5a7b87c50a52cedae98021425a5121429
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6049400
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Auto-Submit: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1393165}
  • Loading branch information
josepharhar authored and Chromium LUCI CQ committed Dec 6, 2024
1 parent d2a28a9 commit ad30945
Show file tree
Hide file tree
Showing 13 changed files with 20 additions and 60 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/select_popup_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
class SelectPopupBrowsertest : public InProcessBrowserTest {
public:
SelectPopupBrowsertest() {
feature_list_.InitAndEnableFeature(blink::features::kCSSPseudoOpenClosed);
feature_list_.InitAndEnableFeature(blink::features::kCSSPseudoOpen);
}

void TestBody() override {}
Expand Down
9 changes: 2 additions & 7 deletions third_party/blink/renderer/core/css/css_selector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,6 @@ PseudoId CSSSelector::GetPseudoId(PseudoType type) {
case kPseudoAutofillSelected:
case kPseudoBlinkInternalElement:
case kPseudoChecked:
case kPseudoClosed:
case kPseudoCornerPresent:
case kPseudoCue:
case kPseudoCurrent:
Expand Down Expand Up @@ -573,7 +572,6 @@ constexpr static NameToPseudoStruct kPseudoTypeWithoutArgumentsMap[] = {
{"before", CSSSelector::kPseudoBefore},
{"checked", CSSSelector::kPseudoChecked},
{"checkmark", CSSSelector::kPseudoCheckMark},
{"closed", CSSSelector::kPseudoClosed},
{"column", CSSSelector::kPseudoColumn},
{"corner-present", CSSSelector::kPseudoCornerPresent},
{"cue", CSSSelector::kPseudoWebKitCustomElement},
Expand Down Expand Up @@ -773,9 +771,8 @@ CSSSelector::PseudoType CSSSelector::NameToPseudoType(
return CSSSelector::kPseudoUnknown;
}

if ((match->type == CSSSelector::kPseudoOpen ||
match->type == CSSSelector::kPseudoClosed) &&
!RuntimeEnabledFeatures::CSSPseudoOpenClosedEnabled()) {
if (match->type == CSSSelector::kPseudoOpen &&
!RuntimeEnabledFeatures::CSSPseudoOpenEnabled()) {
return CSSSelector::kPseudoUnknown;
}

Expand Down Expand Up @@ -935,7 +932,6 @@ void CSSSelector::UpdatePseudoType(const AtomicString& value,
case kPseudoAutofillPreviewed:
case kPseudoAutofillSelected:
case kPseudoChecked:
case kPseudoClosed:
case kPseudoCornerPresent:
case kPseudoCurrent:
case kPseudoDecrement:
Expand Down Expand Up @@ -1712,7 +1708,6 @@ bool CSSSelector::IsAllowedAfterPart() const {
case kPseudoPictureInPicture:
case kPseudoPlaying:
case kPseudoXrOverlay:
case kPseudoClosed:
case kPseudoDefined:
case kPseudoDir:
case kPseudoFutureCue:
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/renderer/core/css/css_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,6 @@ class CORE_EXPORT CSSSelector {
kPseudoWebKitCustomElement,
// Pseudo elements in UA ShadowRoots. Available only in UA stylesheets.
kPseudoBlinkInternalElement,
kPseudoClosed,
// Pseudo element for fragment styling
kPseudoColumn,
kPseudoCue,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ bool SupportsInvalidation(CSSSelector::PseudoType type) {
case CSSSelector::kPseudoMultiSelectFocus:
case CSSSelector::kPseudoHostHasNonAutoAppearance:
case CSSSelector::kPseudoOpen:
case CSSSelector::kPseudoClosed:
case CSSSelector::kPseudoDialogInTopLayer:
case CSSSelector::kPseudoPicker:
case CSSSelector::kPseudoPopoverInTopLayer:
Expand Down Expand Up @@ -1646,7 +1645,6 @@ RuleInvalidationDataVisitor<VisitorType>::InvalidationSetForSimpleSelector(
case CSSSelector::kPseudoOutOfRange:
case CSSSelector::kPseudoDefined:
case CSSSelector::kPseudoOpen:
case CSSSelector::kPseudoClosed:
case CSSSelector::kPseudoPopoverOpen:
case CSSSelector::kPseudoVideoPersistent:
case CSSSelector::kPseudoVideoPersistentAncestor:
Expand Down
9 changes: 0 additions & 9 deletions third_party/blink/renderer/core/css/selector_checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2134,15 +2134,6 @@ bool SelectorChecker::CheckPseudoClass(const SelectorCheckingContext& context,
return select->PopupIsVisible();
}
return false;
case CSSSelector::kPseudoClosed:
if (auto* dialog = DynamicTo<HTMLDialogElement>(element)) {
return !dialog->FastHasAttribute(html_names::kOpenAttr);
} else if (auto* details = DynamicTo<HTMLDetailsElement>(element)) {
return !details->FastHasAttribute(html_names::kOpenAttr);
} else if (auto* select = DynamicTo<HTMLSelectElement>(element)) {
return select->UsesMenuList() && !select->PopupIsVisible();
}
return false;
case CSSSelector::kPseudoFullscreen:
// fall through
case CSSSelector::kPseudoFullScreen:
Expand Down
7 changes: 3 additions & 4 deletions third_party/blink/renderer/core/html/forms/select_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ void MenuListSelectType::ShowPopup(PopupMenu::ShowEventType type) {
// an expensive call to popup_->UpdateFromElement in DidRecalcStyle.
if (RuntimeEnabledFeatures::SelectPopupLessUpdatesEnabled()) {
SetNativePopupIsVisible(true);
if (RuntimeEnabledFeatures::CSSPseudoOpenClosedEnabled()) {
if (RuntimeEnabledFeatures::CSSPseudoOpenEnabled()) {
select_->GetDocument().UpdateStyleAndLayoutForNode(
select_, DocumentUpdateReason::kPagePopup);
}
Expand All @@ -757,7 +757,7 @@ void MenuListSelectType::ShowPopup(PopupMenu::ShowEventType type) {

if (!RuntimeEnabledFeatures::SelectPopupLessUpdatesEnabled()) {
SetNativePopupIsVisible(true);
if (RuntimeEnabledFeatures::CSSPseudoOpenClosedEnabled()) {
if (RuntimeEnabledFeatures::CSSPseudoOpenEnabled()) {
select_->GetDocument().UpdateStyleAndLayoutForNode(
select_, DocumentUpdateReason::kPagePopup);
}
Expand Down Expand Up @@ -800,9 +800,8 @@ bool MenuListSelectType::PopupIsVisible() const {

void MenuListSelectType::SetNativePopupIsVisible(bool popup_is_visible) {
native_popup_is_visible_ = popup_is_visible;
if (RuntimeEnabledFeatures::CSSPseudoOpenClosedEnabled()) {
if (RuntimeEnabledFeatures::CSSPseudoOpenEnabled()) {
select_->PseudoStateChanged(CSSSelector::kPseudoOpen);
select_->PseudoStateChanged(CSSSelector::kPseudoClosed);
}
if (auto* layout_object = select_->GetLayoutObject()) {
// Invalidate paint to ensure that the focus ring is updated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ const char* PseudoTypeToString(CSSSelector::PseudoType pseudo_type) {
DEFINE_STRING_MAPPING(PseudoListBox)
DEFINE_STRING_MAPPING(PseudoMultiSelectFocus)
DEFINE_STRING_MAPPING(PseudoOpen)
DEFINE_STRING_MAPPING(PseudoClosed)
DEFINE_STRING_MAPPING(PseudoPicker)
DEFINE_STRING_MAPPING(PseudoDialogInTopLayer)
DEFINE_STRING_MAPPING(PseudoPopoverInTopLayer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1137,9 +1137,9 @@
status: "experimental",
},
{
// Enables the :open and :closed pseudo-selectors.
// Enables the :open pseudo-selector.
// https://chromestatus.com/feature/5085419215781888
name: "CSSPseudoOpenClosed",
name: "CSSPseudoOpen",
status: "experimental",
},
{
Expand Down Expand Up @@ -1328,10 +1328,10 @@
name: "CustomizableSelect",
status: "experimental",
// :open is used in the UA stylesheet for CustomizableSelect, so this feature
// will not work properly without CSSPseudoOpenClosed.
// will not work properly without CSSPseudoOpen.
// SelectParserRelaxation is needed to allow more content in <select>
// than just <option>s etc.
depends_on: ["CSSPseudoOpenClosed", "SelectParserRelaxation"],
depends_on: ["CSSPseudoOpen", "SelectParserRelaxation"],
},
{
name: "Database",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@
-external/wpt/css/selectors/selectors-4/lang-024.html
-external/wpt/css/selectors/invalidation/lang-pseudo-class-in-has-multiple-document-elements.html
-external/wpt/css/selectors/media/media-loading-state.html
-external/wpt/css/selectors/open-closed-pseudo.html
-external/wpt/css/selectors/open-pseudo.html
-external/wpt/css/selectors/remove-hovered-element.html
-external/wpt/css/selectors/user-invalid-form-submission-invalidation.html
-external/wpt/density-size-correction/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
test_valid_selector("::part(mypart):any-link");
test_valid_selector("::part(mypart):autofill");
test_valid_selector("::part(mypart):checked");
test_valid_selector("::part(mypart):closed");
test_valid_selector("::part(mypart):default");
test_valid_selector("::part(mypart):defined");
test_valid_selector("::part(mypart):dir(ltr)");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,53 +17,39 @@
<script>
test(() => {
const dialog = document.querySelector('dialog');
assert_true(dialog.matches(':closed'),
':closed should match when the dialog is closed.');
assert_false(dialog.matches(':open'),
':open should not match when the dialog is closed.');

dialog.show();
assert_false(dialog.matches(':closed'),
':closed should not match after dialog.open().');
assert_true(dialog.matches(':open'),
':open should match after dialog.open().');

dialog.close();
dialog.showModal();
assert_false(dialog.matches(':closed'),
':closed should not match after dialog.showModal().');
assert_true(dialog.matches(':open'),
':open should match after dialog.showModal().');

dialog.close();
}, 'The dialog element should support :open and :closed.');
}, 'The dialog element should support :open.');

test(() => {
const details = document.querySelector('details');
assert_true(details.matches(':closed'),
':closed should match when the details is closed.');
assert_false(details.matches(':open'),
':open should not match when the details is closed.');

details.open = true;
assert_false(details.matches(':closed'),
':closed should not match when the details is open.');
assert_true(details.matches(':open'),
':open should match when the details is open.');
}, 'The details element should support :open and :closed.');
}, 'The details element should support :open.');

promise_test(async () => {
const select = document.querySelector('select');
assert_true(select.matches(':closed'),
':closed should match when the select is closed.');
assert_false(select.matches(':open'),
':open should not match when the select is closed.');

await test_driver.click(select);
await new Promise(requestAnimationFrame);
assert_false(select.matches(':closed'),
':closed should not match when the select is open.');
assert_true(select.matches(':open'),
':open should match when the select is open.');
}, 'The select element should support :open and :closed.');
}, 'The select element should support :open.');
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
select {
background-color: rgb(0, 0, 255);
}
select:closed {
select:not(:open) {
background-color: rgb(0, 255, 0);
}
select:open {
Expand All @@ -29,22 +29,22 @@
<script>
promise_test(async () => {
assert_equals(getComputedStyle(select).backgroundColor, 'rgb(0, 255, 0)',
'The select should match :closed at the start of the test.');
'The select should match :not(:open) at the start of the test.');

await test_driver.click(select);
assert_equals(getComputedStyle(select).backgroundColor, 'rgb(255, 0, 0)',
'The select should match :open when opened.');

await test_driver.click(opttwo);
assert_equals(getComputedStyle(select).backgroundColor, 'rgb(0, 255, 0)',
'The select should match :closed after clicking an option.');
'The select should match :not(:open) after clicking an option.');

await test_driver.click(select);
assert_equals(getComputedStyle(select).backgroundColor, 'rgb(255, 0, 0)',
'The select should match :open when reopened.');

await test_driver.click(button);
assert_equals(getComputedStyle(select).backgroundColor, 'rgb(0, 255, 0)',
'The select should match :closed after light dismiss.');
'The select should match :not(:open) after light dismiss.');
}, 'select should not match :open when light dismissed.');
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,20 @@
const select = document.getElementById('single');
assert_false(internals.isSelectPopupVisible(select),
'internals.isSelectPopupVisibile should be false while popup is closed.');
assert_true(select.matches(':closed'),
':closed should match while popup is closed.');
assert_false(select.matches(':open'),
':open should not match while popup is closed.');

select.focus();
eventSender.keyDown(' ');
assert_true(internals.isSelectPopupVisible(select),
'internals.isSelectPopupVisible should be true while the popup is open.');
assert_false(select.matches(':closed'),
':closed should not match while popup is open.');
assert_true(select.matches(':open'),
':open should match while popup is open.');
}, 'Single-select with popup should support :open and :closed.');
}, 'Single-select with popup should support :open.');

test(() => {
const select = document.getElementById('multiple');
assert_false(select.matches(':closed'),
':closed should not match on <select mulitple>.');
assert_false(select.matches(':open'),
':open should not match on <select multiple>.');
}, 'Multi-select without popup should not support :open or :closed.');
}, 'Multi-select without popup should not support :open.');
</script>

0 comments on commit ad30945

Please sign in to comment.