Remove display:block!important from customizable_select.css

1. Call UpdateStyleAndLayoutTree from IsAppearanceBaseButton and
   IsAppearanceBasePicker, unless no_update is set for cases which are
   calling from within a style/layout recalc.
2. Call EnsureComputedStyle when checking IsAppearanceBasePicker in
   order to get an appearance value despite the element being
   display:none due to it being a closed popover.

Since the picker might no longer have a computed style all the time,
this requires rewriting some accessibility code which was depending on
always having a computed style for the picker. I rewrote the
accessibility code to be more consistent across appearance:base-select
mode and appearance:auto mode by consistently using the UA popover
element as the MenuListPopup mapped element.

Bug: 364348901
Fixed: 364924715
Change-Id: I3fec57f2e02ce4200c95c256e5c1072b5c21ef8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5953628
Reviewed-by: David Baron <dbaron@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399296}

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -198,6 +198,45 @@ class SummaryAsHeadingDumpAccessibilityTreeTest
   base::test::ScopedFeatureList feature_list_;
 };
 
+class CustomizableSelectEnabledDumpAccessibilityTreeTest
+    : public DumpAccessibilityTreeTest {
+ protected:
+  CustomizableSelectEnabledDumpAccessibilityTreeTest() {
+    feature_list_.InitWithFeatures({{blink::features::kCustomizableSelect,
+                                     blink::features::kSelectParserRelaxation}},
+                                   {/* disabled_features */});
+  }
+
+  ~CustomizableSelectEnabledDumpAccessibilityTreeTest() override {
+    // Ensure that the feature lists are destroyed in the same order they
+    // were created in.
+    scoped_feature_list_.Reset();
+    feature_list_.Reset();
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+class CustomizableSelectDisabledDumpAccessibilityTreeTest
+    : public DumpAccessibilityTreeTest {
+ protected:
+  CustomizableSelectDisabledDumpAccessibilityTreeTest() {
+    feature_list_.InitWithFeatures({/* enabled_features */},
+                                   {{blink::features::kCustomizableSelect}});
+  }
+
+  ~CustomizableSelectDisabledDumpAccessibilityTreeTest() override {
+    // Ensure that the feature lists are destroyed in the same order they
+    // were created in.
+    scoped_feature_list_.Reset();
+    feature_list_.Reset();
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
 // TODO(crbug.com/40925629): We need to create a way to incrementally
 // enable and create UIA tests.
 INSTANTIATE_TEST_SUITE_P(
@@ -224,6 +263,18 @@ INSTANTIATE_TEST_SUITE_P(
     ::testing::ValuesIn(DumpAccessibilityTestBase::TreeTestPasses()),
     DumpAccessibilityTreeTestPassToString());
 
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    CustomizableSelectEnabledDumpAccessibilityTreeTest,
+    ::testing::ValuesIn(DumpAccessibilityTestBase::TreeTestPasses()),
+    DumpAccessibilityTreeTestPassToString());
+
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    CustomizableSelectDisabledDumpAccessibilityTreeTest,
+    ::testing::ValuesIn(DumpAccessibilityTestBase::TreeTestPasses()),
+    DumpAccessibilityTreeTestPassToString());
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityCSSAltText) {
   RunCSSTest(FILE_PATH_LITERAL("alt-text.html"));
 }
@@ -2156,6 +2207,11 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
   RunHtmlTest(FILE_PATH_LITERAL("custom-select-open.html"));
 }
 
+IN_PROC_BROWSER_TEST_P(CustomizableSelectEnabledDumpAccessibilityTreeTest,
+                       AccessibilityCustomSelectMixedOptions) {
+  RunHtmlTest(FILE_PATH_LITERAL("custom-select-mixed-options.html"));
+}
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityDd) {
   RunHtmlTest(FILE_PATH_LITERAL("dd.html"));
 }
@@ -3516,7 +3572,18 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
   RunHtmlTest(FILE_PATH_LITERAL("selection-container.html"));
 }
 
-IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilitySelect) {
+// Times out on android: https://issues.chromium.org/issues/384959329
+#if BUILDFLAG(IS_ANDROID)
+#define MAYBE_AccessibilitySelect DISABLED_AccessibiltySelect
+#else
+#define MAYBE_AccessibilitySelect ENABLED_AccessibiltySelect
+#endif
+IN_PROC_BROWSER_TEST_P(CustomizableSelectEnabledDumpAccessibilityTreeTest,
+                       MAYBE_AccessibilitySelect) {
+  RunHtmlTest(FILE_PATH_LITERAL("select.html"));
+}
+IN_PROC_BROWSER_TEST_P(CustomizableSelectDisabledDumpAccessibilityTreeTest,
+                       AccessibilitySelect) {
   RunHtmlTest(FILE_PATH_LITERAL("select.html"));
 }
 
@@ -3526,7 +3593,11 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilitySelect) {
 #else
 #define MAYBE_AccessibilitySelectOpen AccessibilitySelectOpen
 #endif
-IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
+IN_PROC_BROWSER_TEST_P(CustomizableSelectEnabledDumpAccessibilityTreeTest,
+                       MAYBE_AccessibilitySelectOpen) {
+  RunHtmlTest(FILE_PATH_LITERAL("select-open.html"));
+}
+IN_PROC_BROWSER_TEST_P(CustomizableSelectDisabledDumpAccessibilityTreeTest,
                        MAYBE_AccessibilitySelectOpen) {
   RunHtmlTest(FILE_PATH_LITERAL("select-open.html"));
 }
@@ -3540,18 +3611,31 @@ IN_PROC_BROWSER_TEST_P(YieldingParserDumpAccessibilityTreeTest,
   RunHtmlTest(FILE_PATH_LITERAL("select-in-canvas.html"));
 }
 
-IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
+IN_PROC_BROWSER_TEST_P(CustomizableSelectEnabledDumpAccessibilityTreeTest,
+                       AccessibilitySelectFollowsFocus) {
+  RunHtmlTest(FILE_PATH_LITERAL("select-follows-focus.html"));
+}
+IN_PROC_BROWSER_TEST_P(CustomizableSelectDisabledDumpAccessibilityTreeTest,
                        AccessibilitySelectFollowsFocus) {
   RunHtmlTest(FILE_PATH_LITERAL("select-follows-focus.html"));
 }
 
-IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
+IN_PROC_BROWSER_TEST_P(CustomizableSelectEnabledDumpAccessibilityTreeTest,
+                       AccessibilitySelectFollowsFocusAriaSelectedFalse) {
+  RunHtmlTest(
+      FILE_PATH_LITERAL("select-follows-focus-aria-selected-false.html"));
+}
+IN_PROC_BROWSER_TEST_P(CustomizableSelectDisabledDumpAccessibilityTreeTest,
                        AccessibilitySelectFollowsFocusAriaSelectedFalse) {
   RunHtmlTest(
       FILE_PATH_LITERAL("select-follows-focus-aria-selected-false.html"));
 }
 
-IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
+IN_PROC_BROWSER_TEST_P(CustomizableSelectEnabledDumpAccessibilityTreeTest,
+                       AccessibilitySelectFollowsFocusMultiselect) {
+  RunHtmlTest(FILE_PATH_LITERAL("select-follows-focus-multiselect.html"));
+}
+IN_PROC_BROWSER_TEST_P(CustomizableSelectDisabledDumpAccessibilityTreeTest,
                        AccessibilitySelectFollowsFocusMultiselect) {
   RunHtmlTest(FILE_PATH_LITERAL("select-follows-focus-multiselect.html"));
 }

content/test/content_test_bundle_data.filelist

@@ -3344,6 +3344,9 @@ data/accessibility/html/custom-select-expected-blink.txt
 data/accessibility/html/custom-select-expected-mac.txt
 data/accessibility/html/custom-select-expected-uia-win.txt
 data/accessibility/html/custom-select-expected-win.txt
+data/accessibility/html/custom-select-mixed-options-expected-auralinux.txt
+data/accessibility/html/custom-select-mixed-options-expected-blink.txt
+data/accessibility/html/custom-select-mixed-options.html
 data/accessibility/html/custom-select-open-expected-android-assist-data.txt
 data/accessibility/html/custom-select-open-expected-android-external.txt
 data/accessibility/html/custom-select-open-expected-android.txt

content/test/data/accessibility/html/custom-select-mixed-options-expected-auralinux.txt

@@ -0,0 +1,18 @@
+[document web]
+++[section]
+++++[combo box]
+++++++[menu]
+++++++++[menu item] name='only text' selectable selected
+++++++++[menu item] name='with img ' selectable
+++++[combo box]
+++++++[menu]
+++++++++[menu item] name='only text' selectable selected
+++++++++[menu item] name='with img ' selectable
+++++++++++[static] name='with img<newline>    '
+++++++++++[image]
+++++[combo box]
+++++++[menu]
+++++++++[menu item] name='only text' selectable selected
+++++++++[menu item] name='with img ' selectable
+++++++++++[static] name='with img<newline>    '
+++++++++++[image]

content/test/data/accessibility/html/custom-select-mixed-options-expected-blink.txt

@@ -0,0 +1,21 @@
+rootWebArea
+++genericContainer ignored
+++++genericContainer
+++++++comboBoxSelect collapsed value='only text'
+++++++++menuListPopup invisible ispopup=auto
+++++++++++menuListOption name='only text' selected=true
+++++++++++menuListOption invisible name='with img ' selected=false
+++++++comboBoxSelect collapsed value='only text'
+++++++++menuListPopup invisible ispopup=auto
+++++++++++menuListOption name='only text' selected=true
+++++++++++menuListOption invisible name='with img ' selected=false
+++++++++++++genericContainer ignored invisible
+++++++++++++++staticText invisible name='with img<newline>    '
+++++++++++++++image invisible
+++++++comboBoxSelect collapsed value='only text'
+++++++++menuListPopup invisible ispopup=auto
+++++++++++menuListOption name='only text' selected=true
+++++++++++menuListOption invisible name='with img ' selected=false
+++++++++++++genericContainer ignored invisible
+++++++++++++++staticText invisible name='with img<newline>    '
+++++++++++++++image invisible

content/test/data/accessibility/html/custom-select-mixed-options.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<style>
+.custombutton {
+  appearance: base-select;
+}
+.custompicker::picker(select) {
+  appearance: base-select;
+}
+</style>
+
+<select>
+  <option>only text</option>
+  <option>with img
+    <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAYAAACNiR0NAAAAQklEQVQ4T2P8z8AARIQBI1GqGBhA6ohSOmogzkAfDUPc6XHgkg0wXROVsIF5gHB2AqoAqho1EHceGA1D7CEw+JMNALGaMffRFnRtAAAAAElFTkSuQmCC">
+  </option>
+</select>
+
+<select class=custombutton>
+  <option>only text</option>
+  <option>with img
+    <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAYAAACNiR0NAAAAQklEQVQ4T2P8z8AARIQBI1GqGBhA6ohSOmogzkAfDUPc6XHgkg0wXROVsIF5gHB2AqoAqho1EHceGA1D7CEw+JMNALGaMffRFnRtAAAAAElFTkSuQmCC">
+  </option>
+</select>
+
+<select class="custombutton custompicker">
+  <option>only text</option>
+  <option>with img
+    <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAYAAACNiR0NAAAAQklEQVQ4T2P8z8AARIQBI1GqGBhA6ohSOmogzkAfDUPc6XHgkg0wXROVsIF5gHB2AqoAqho1EHceGA1D7CEw+JMNALGaMffRFnRtAAAAAElFTkSuQmCC">
+  </option>
+</select>

third_party/blink/public/common/metrics/document_update_reason.h

@@ -18,6 +18,7 @@ namespace blink {
 enum class DocumentUpdateReason {
   kAccessibility,
   kBaseColor,
+  kBaseSelect,
   kBeginMainFrame,
   kCanvas,
   kCanvasPlaceElement,

third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.cc

@@ -685,6 +685,7 @@ void LocalFrameUkmAggregator::EndForcedLayout(
 
     case DocumentUpdateReason::kAccessibility:
     case DocumentUpdateReason::kBaseColor:
+    case DocumentUpdateReason::kBaseSelect:
     case DocumentUpdateReason::kComputedStyle:
     case DocumentUpdateReason::kDisplayLock:
     case DocumentUpdateReason::kViewTransition:

third_party/blink/renderer/core/html/forms/html_option_element.cc

@@ -130,7 +130,8 @@ FocusableState HTMLOptionElement::SupportsFocus(
     UpdateBehavior update_behavior) const {
   HTMLSelectElement* select = OwnerSelectElement();
   if (select && select->UsesMenuList()) {
-    if (select->IsAppearanceBasePicker()) {
+    auto* popover = select->PopoverForAppearanceBase();
+    if (popover && popover->popoverOpen()) {
       // If this option is being rendered as regular web content inside a
       // base-select <select> popover, then we need this element to be
       // focusable.

third_party/blink/renderer/core/html/forms/html_select_element.cc

@@ -106,7 +106,8 @@ class SelectDescendantsObserver : public MutationObserver::Delegate {
   explicit SelectDescendantsObserver(HTMLSelectElement& select)
       : select_(select), observer_(MutationObserver::Create(this)) {
     CHECK(RuntimeEnabledFeatures::CustomizableSelectEnabled());
-    DCHECK(select_->IsAppearanceBasePicker());
+    DCHECK(select_->IsAppearanceBaseButton(
+        HTMLSelectElement::StyleUpdateBehavior::kDontUpdateStyle));
 
     MutationObserverInit* init = MutationObserverInit::Create();
     init->setChildList(true);
@@ -1410,7 +1411,8 @@ void HTMLSelectElement::UpdateMutationObserver() {
   if (!RuntimeEnabledFeatures::CustomizableSelectEnabled()) {
     return;
   }
-  if (UsesMenuList() && isConnected() && IsAppearanceBasePicker()) {
+  if (UsesMenuList() && isConnected() &&
+      IsAppearanceBaseButton(StyleUpdateBehavior::kDontUpdateStyle)) {
     if (!descendants_observer_) {
       descendants_observer_ =
           MakeGarbageCollected<SelectDescendantsObserver>(*this);
@@ -1962,6 +1964,14 @@ HTMLElement* HTMLSelectElement::PopoverForAppearanceBase() const {
   return select_type_->PopoverForAppearanceBase();
 }
 
+// static
+bool HTMLSelectElement::IsPopoverForAppearanceBase(const Node* node) {
+  if (auto* element = DynamicTo<Element>(node)) {
+    return IsPopoverForAppearanceBase(element);
+  }
+  return false;
+}
+
 // static
 bool HTMLSelectElement::IsPopoverForAppearanceBase(const Element* element) {
   if (auto* root = DynamicTo<ShadowRoot>(element->parentNode())) {
@@ -1971,8 +1981,9 @@ bool HTMLSelectElement::IsPopoverForAppearanceBase(const Element* element) {
   return false;
 }
 
-bool HTMLSelectElement::IsAppearanceBaseButton() const {
+bool HTMLSelectElement::IsAppearanceBaseButton(
-  return select_type_->IsAppearanceBaseButton();
+    StyleUpdateBehavior update_behavior) const {
+  return select_type_->IsAppearanceBaseButton(update_behavior);
 }
 
 bool HTMLSelectElement::IsAppearanceBasePicker() const {
@@ -2099,8 +2110,10 @@ void HTMLSelectElement::UpdateAllSelectedcontents() {
        VectorOf<HTMLSelectedContentElement>(descendant_selectedcontents_)) {
     selectedcontent->CloneContentsFromOptionElement(option);
   }
-  if (auto* attr_selectedcontent = selectedContentElement()) {
+  if (RuntimeEnabledFeatures::SelectedcontentelementAttributeEnabled()) {
-    attr_selectedcontent->CloneContentsFromOptionElement(option);
+    if (auto* attr_selectedcontent = selectedContentElement()) {
+      attr_selectedcontent->CloneContentsFromOptionElement(option);
+    }
   }
 }
 

third_party/blink/renderer/core/html/forms/html_select_element.h

@@ -245,6 +245,7 @@ class CORE_EXPORT HTMLSelectElement final
 
   // Returns true if the provided element is some select element's
   // PopoverForAppearanceBase.
+  static bool IsPopoverForAppearanceBase(const Node*);
   static bool IsPopoverForAppearanceBase(const Element*);
 
   // <select> supports appearance:base-select on both the main element and
@@ -255,7 +256,14 @@ class CORE_EXPORT HTMLSelectElement final
   // has appearance:base-select, IsAppearanceBasePicker will return true and the
   // popup will be a popover element. The SelectType must also support base
   // appearance, which is currently only MenuListSelectType.
-  bool IsAppearanceBaseButton() const;
+  // IsAppearanceBaseButton should be used for code which is concerned with the
+  // in-page rendering of the button, and IsAppearanceBasePicker should be used
+  // for code which is concerned with the popup/popover and the other elements
+  // which are rendered in it.
+  // |no_update| prevents these methods from running an UpdateStyleAndLayoutTree
+  // which is needed in some cases to prevent nested style/layout recalc.
+  enum class StyleUpdateBehavior { kUpdateStyle, kDontUpdateStyle };
+  bool IsAppearanceBaseButton(StyleUpdateBehavior) const;
   bool IsAppearanceBasePicker() const;
 
   void SelectedContentElementInserted(

third_party/blink/renderer/core/html/forms/select_type.cc

@@ -238,7 +238,8 @@ class MenuListSelectType final : public SelectType {
   void ManuallyAssignSlots() override;
   HTMLButtonElement* SlottedButton() const override;
   HTMLElement* PopoverForAppearanceBase() const override;
-  bool IsAppearanceBaseButton() const override;
+  bool IsAppearanceBaseButton(
+      HTMLSelectElement::StyleUpdateBehavior) const override;
   bool IsAppearanceBasePicker() const override;
   HTMLSelectElement::SelectAutofillPreviewElement* GetAutofillPreviewElement()
       const override;
@@ -409,7 +410,9 @@ bool MenuListSelectType::DefaultEventHandler(const Event& event) {
     // TODO(crbug.com/1511354): Reconsider making appearance:base-select affect
     // keyboard behavior after a resolution here:
     // https://github.com/openui/open-ui/issues/1087
-    if (IsAppearanceBaseButton() && key_code == '\r') {
+    if (IsAppearanceBaseButton(
+            HTMLSelectElement::StyleUpdateBehavior::kUpdateStyle) &&
+        key_code == '\r') {
       // TODO(crbug.com/1511354): Consider making form->SubmitImplicitly work
       // here instead of PrepareForSubmission and combine with the subsequent
       // code.
@@ -484,7 +487,8 @@ bool MenuListSelectType::ShouldOpenPopupForKeyDownEvent(
   // TODO(crbug.com/1511354): Reconsider making appearance:base-select affect
   // keyboard behavior after a resolution here:
   // https://github.com/openui/open-ui/issues/1087
-  if (IsAppearanceBaseButton() &&
+  if (IsAppearanceBaseButton(
+          HTMLSelectElement::StyleUpdateBehavior::kUpdateStyle) &&
       (key == keywords::kArrowDown || key == keywords::kArrowUp ||
        key == keywords::kArrowLeft || key == keywords::kArrowRight)) {
     return true;
@@ -505,7 +509,9 @@ bool MenuListSelectType::ShouldOpenPopupForKeyPressEvent(
   // TODO(crbug.com/1511354): Reconsider making appearance:base-select affect
   // keyboard behavior after a resolution here:
   // https://github.com/openui/open-ui/issues/1087
-  if (IsAppearanceBaseButton() && key_code == '\r') {
+  if (IsAppearanceBaseButton(
+          HTMLSelectElement::StyleUpdateBehavior::kUpdateStyle) &&
+      key_code == '\r') {
     return false;
   }
 
@@ -648,22 +654,20 @@ HTMLButtonElement* MenuListSelectType::SlottedButton() const {
 }
 
 HTMLElement* MenuListSelectType::PopoverForAppearanceBase() const {
-  // LayoutFlexibleBox::IsChildAllowed needs to access popover_ even when the
+  CHECK(RuntimeEnabledFeatures::CustomizableSelectEnabled() || !popover_);
-  // author doesn't put appearance:base-select on ::picker(select). In order to
-  // return popover_ in this case, we check IsAppearanceBaseButton instead of
-  // IsAppearanceBaseSelect.
-  if (!IsAppearanceBaseButton()) {
-    return nullptr;
-  }
   return popover_;
 }
 
-bool MenuListSelectType::IsAppearanceBaseButton() const {
+bool MenuListSelectType::IsAppearanceBaseButton(
+    HTMLSelectElement::StyleUpdateBehavior update_behavior) const {
   if (!RuntimeEnabledFeatures::CustomizableSelectEnabled()) {
     return false;
   }
-  // TODO(crbug.com/364348901): Update style and layout here.
   DCHECK(select_);
+  if (update_behavior == HTMLSelectElement::StyleUpdateBehavior::kUpdateStyle) {
+    select_->GetDocument().UpdateStyleAndLayoutForNode(
+        select_, DocumentUpdateReason::kBaseSelect);
+  }
   if (auto* style = select_->GetComputedStyle()) {
     return style->EffectiveAppearance() == AppearanceValue::kBaseSelect;
   }
@@ -671,17 +675,15 @@ bool MenuListSelectType::IsAppearanceBaseButton() const {
 }
 
 bool MenuListSelectType::IsAppearanceBasePicker() const {
-  if (!IsAppearanceBaseButton()) {
+  if (!IsAppearanceBaseButton(
+          HTMLSelectElement::StyleUpdateBehavior::kUpdateStyle)) {
     // The author is required to put appearance:base-select on the <select>
     // before the ::picker is allowed to have appearance:base-select.
     return false;
   }
   CHECK(RuntimeEnabledFeatures::CustomizableSelectEnabled());
-  // TODO(crbug.com/364348901): Consider using EnsureComputedStyle() here to get
-  // more reliable results, though it has the risk of causing more style
-  // computation, sometimes at bad times.
   DCHECK(popover_);
-  if (auto* style = popover_->GetComputedStyle()) {
+  if (auto* style = popover_->EnsureComputedStyle()) {
     return style->EffectiveAppearance() == AppearanceValue::kBaseSelect;
   }
   return false;
@@ -780,11 +782,16 @@ void MenuListSelectType::PopupDidHide() {
 }
 
 bool MenuListSelectType::PopupIsVisible() const {
-  if (IsAppearanceBasePicker()) {
+  if (RuntimeEnabledFeatures::CustomizableSelectEnabled()) {
-    return popover_->popoverOpen();
+    // This is called during style recalc, so we can't check
-  } else {
+    // IsAppearanceBasePicker to determine which picker to look at.
-    return native_popup_is_visible_;
+    bool popover_open = popover_->popoverOpen();
+    CHECK(!(popover_open && native_popup_is_visible_))
+        << " The base appearance and native pickers should never both be open "
+           "at the same time.";
+    return popover_open || native_popup_is_visible_;
   }
+  return native_popup_is_visible_;
 }
 
 void MenuListSelectType::SetNativePopupIsVisible(bool popup_is_visible) {
@@ -878,7 +885,8 @@ void MenuListSelectType::DidSetSuggestedOption(HTMLOptionElement* option) {
   if (native_popup_is_visible_) {
     popup_->UpdateFromElement(PopupMenu::kBySelectionChange);
   }
-  if (IsAppearanceBaseButton()) {
+  if (IsAppearanceBaseButton(
+          HTMLSelectElement::StyleUpdateBehavior::kUpdateStyle)) {
     if (option) {
       autofill_popover_->ShowPopoverInternal(select_, &ASSERT_NO_EXCEPTION);
       autofill_popover_text_->setInnerText(option->label());
@@ -973,7 +981,8 @@ String MenuListSelectType::UpdateTextStyleInternal() {
   // TODO(crbug.com/1511354): Ensure that this runs after switching appearance
   // modes or consider splitting InnerElement into two elements, one for
   // appearance:base-select and one for appearance:auto.
-  if (!IsAppearanceBaseButton()) {
+  if (!IsAppearanceBaseButton(
+          HTMLSelectElement::StyleUpdateBehavior::kDontUpdateStyle)) {
     Element& inner_element = select_->InnerElement();
     const ComputedStyle* inner_style = inner_element.GetComputedStyle();
     if (inner_style && option_style &&
@@ -1040,7 +1049,9 @@ HTMLOptionElement* MenuListSelectType::OptionToBeShown() const {
     return option;
   // In appearance:base-select mode, we don't want to reveal the suggested
   // option anywhere except in autofill_popover_.
-  if (select_->suggested_option_ && !IsAppearanceBaseButton()) {
+  if (select_->suggested_option_ &&
+      !IsAppearanceBaseButton(
+          HTMLSelectElement::StyleUpdateBehavior::kDontUpdateStyle)) {
     return select_->suggested_option_.Get();
   }
   // TODO(tkent): We should not call OptionToBeShown() in IsMultiple() case.
@@ -1164,7 +1175,8 @@ class ListBoxSelectType final : public SelectType {
   void ManuallyAssignSlots() override;
   HTMLButtonElement* SlottedButton() const override;
   HTMLElement* PopoverForAppearanceBase() const override;
-  bool IsAppearanceBaseButton() const override;
+  bool IsAppearanceBaseButton(
+      HTMLSelectElement::StyleUpdateBehavior) const override;
   bool IsAppearanceBasePicker() const override;
   HTMLSelectElement::SelectAutofillPreviewElement* GetAutofillPreviewElement()
       const override;
@@ -1837,7 +1849,8 @@ HTMLElement* ListBoxSelectType::PopoverForAppearanceBase() const {
   return nullptr;
 }
 
-bool ListBoxSelectType::IsAppearanceBaseButton() const {
+bool ListBoxSelectType::IsAppearanceBaseButton(
+    HTMLSelectElement::StyleUpdateBehavior) const {
   return false;
 }
 

third_party/blink/renderer/core/html/forms/select_type.h

@@ -65,7 +65,8 @@ class SelectType : public GarbageCollected<SelectType> {
   virtual void ManuallyAssignSlots() = 0;
   virtual HTMLButtonElement* SlottedButton() const = 0;
   virtual HTMLElement* PopoverForAppearanceBase() const = 0;
-  virtual bool IsAppearanceBaseButton() const = 0;
+  virtual bool IsAppearanceBaseButton(
+      HTMLSelectElement::StyleUpdateBehavior) const = 0;
   virtual bool IsAppearanceBasePicker() const = 0;
   virtual HTMLSelectElement::SelectAutofillPreviewElement*
   GetAutofillPreviewElement() const = 0;

third_party/blink/renderer/core/html/resources/customizable_select.css

@@ -85,20 +85,6 @@ select:not(:-internal-list-box)::picker(select) {
     block-start span-inline-start;
 }
 
-/* This rule is here to ensure that we can get a ComputedStyle for
- * ::picker(select). Without this, the [popover] UA style would make it
- * display:none, which would mean that we wouldn't get a ComputedStyle, which
- * would mean that we can't detect the author opting in with
- * appearance:base-select on this element. This element is hidden when it isn't
- * popover open via LayoutFlexibleBox::IsChildAllowed. This also means that the
- * author can't force it to become visible while it is popover closed by
- * putting display:block on it, but hopefully that's OK.
- * TODO(crbug.com/364348901): Remove this.
- */
-select::picker(select) {
-  display: block !important;
-}
-
 select:not(:-internal-list-box) option {
   /* min-size rules ensure that we meet accessibility guidelines for minimum target size.
    * https://github.com/openui/open-ui/issues/1026

third_party/blink/renderer/core/layout/flex/layout_flexible_box.cc

@@ -87,7 +87,8 @@ bool LayoutFlexibleBox::IsChildAllowed(LayoutObject* object,
                                        const ComputedStyle& style) const {
   const auto* select = DynamicTo<HTMLSelectElement>(GetNode());
   if (select && select->UsesMenuList()) [[unlikely]] {
-    if (select->IsAppearanceBaseButton()) {
+    if (select->IsAppearanceBaseButton(
+            HTMLSelectElement::StyleUpdateBehavior::kDontUpdateStyle)) {
       CHECK(RuntimeEnabledFeatures::CustomizableSelectEnabled());
       if (IsA<HTMLOptionElement>(object->GetNode()) ||
           IsA<HTMLOptGroupElement>(object->GetNode()) ||
@@ -105,14 +106,6 @@ bool LayoutFlexibleBox::IsChildAllowed(LayoutObject* object,
         // the InnerElement.
         return false;
       }
-      if (auto* popover = select->PopoverForAppearanceBase()) {
-        if (child == popover && !popover->popoverOpen()) {
-          // This is needed in order to keep the popover hidden after the UA
-          // sheet is forcing it to be display:block in order to get a computed
-          // style.
-          return false;
-        }
-      }
       return true;
     } else {
       // For a size=1 appearance:auto <select>, we only render the active option

third_party/blink/renderer/core/layout/generated_children.h

@@ -23,7 +23,8 @@ static bool CanHaveGeneratedChildren(const LayoutObject& layout_object) {
     if (RuntimeEnabledFeatures::CustomizableSelectEnabled() &&
         layout_object.IsMenuList() &&
         To<HTMLSelectElement>(layout_object.GetNode())
-            ->IsAppearanceBaseButton()) {
+            ->IsAppearanceBaseButton(
+                HTMLSelectElement::StyleUpdateBehavior::kDontUpdateStyle)) {
       // appearance:base-select <select>s should be allowed to have ::after etc.
       return true;
     }

third_party/blink/renderer/core/layout/layout_box.cc

@@ -1211,7 +1211,8 @@ LayoutUnit LayoutBox::DefaultIntrinsicContentInlineSize() const {
 
   const bool apply_fixed_size = StyleRef().ApplyControlFixedSize(&element);
   const auto* select = DynamicTo<HTMLSelectElement>(element);
-  if (select && select->UsesMenuList() && !select->IsAppearanceBaseButton())
+  if (select && select->UsesMenuList() &&
+      StyleRef().EffectiveAppearance() != AppearanceValue::kBaseSelect)
       [[unlikely]] {
     return apply_fixed_size ? MenuListIntrinsicInlineSize(*select, *this)
                             : kIndefiniteSize;
@@ -1265,12 +1266,11 @@ LayoutUnit LayoutBox::DefaultIntrinsicContentBlockSize() const {
     return kIndefiniteSize;
   }
   if (const auto* select = DynamicTo<HTMLSelectElement>(GetNode())) {
-    if (!select->IsAppearanceBaseButton()) {
+    if (!select->UsesMenuList()) {
-      if (select->UsesMenuList()) {
-        return MenuListIntrinsicBlockSize(*select, *this);
-      }
       return ListBoxItemBlockSize(*select, *this) * select->ListBoxSize() -
              ComputeLogicalScrollbars().BlockSum();
+    } else if (effective_appearance != AppearanceValue::kBaseSelect) {
+      return MenuListIntrinsicBlockSize(*select, *this);
     }
   }
   if (IsTextField()) {

third_party/blink/renderer/modules/accessibility/ax_node_object.cc

@@ -2242,6 +2242,9 @@ ax::mojom::blink::Role AXNodeObject::NativeRoleIgnoringAria() const {
 
   if (ParentObjectIfPresent() && ParentObjectIfPresent()->RoleValue() ==
                                      ax::mojom::blink::Role::kComboBoxSelect) {
+    // Only the UA popover element should get the MenuListPopup role.
+    CHECK(!RuntimeEnabledFeatures::CustomizableSelectEnabled() ||
+          HTMLSelectElement::IsPopoverForAppearanceBase(GetNode()));
     return ax::mojom::blink::Role::kMenuListPopup;
   }
 
@@ -5768,9 +5771,10 @@ void AXNodeObject::AddNodeChildren() {
 void AXNodeObject::AddMenuListChildren() {
   auto* select = To<HTMLSelectElement>(GetNode());
 
-  if (select->IsAppearanceBasePicker()) {
+  if (RuntimeEnabledFeatures::CustomizableSelectEnabled()) {
-    // In appearance: base-select (customizable select), the children of the
+    CHECK(select->UsesMenuList());
-    // combobox is the displayed data list.
+    // When CustomizableSelect is enabled, there will always be one
+    // MenuListPopup child which is the UA popover element.
     AddNodeChild(select->PopoverForAppearanceBase());
     return;
   }
@@ -5781,9 +5785,8 @@ void AXNodeObject::AddMenuListChildren() {
 void AXNodeObject::AddMenuListPopupChildren() {
   auto* select = To<HTMLSelectElement>(ParentObject()->GetNode());
 
-  if (select->IsAppearanceBasePicker()) {
+  // With CustomizableSelect, MenuListPopups can have more interesting children.
-    // In appearance: base-select (customizable select), the children of the
+  if (RuntimeEnabledFeatures::CustomizableSelectEnabled()) {
-    // popup are all of the natural dom children of the <select>.
     for (Node* child = NodeTraversal::FirstChild(*select); child;
          child = NodeTraversal::NextSibling(*child)) {
       if (child == select->SlottedButton()) {
@@ -5797,8 +5800,6 @@ void AXNodeObject::AddMenuListPopupChildren() {
     return;
   }
 
-  // In appearance: auto/none, the children of the popup are the flat tree
-  // children of the slot associated with the popup.
   AddNodeChildren();
 }
 
@@ -5847,9 +5848,18 @@ void AXNodeObject::AddChildrenImpl() {
     AddValidationMessageChild();
   CHECK_ATTACHED();
 
-  if (RoleValue() == ax::mojom::blink::Role::kComboBoxSelect) {
+  auto* select = DynamicTo<HTMLSelectElement>(GetNode());
+  if (RuntimeEnabledFeatures::CustomizableSelectEnabled() && select &&
+      select->UsesMenuList() && !select->IsMultiple()) {
+    // When CustomizableSelect is enabled, then we need to enforce our custom AX
+    // tree structure for select elements even if the author changed the select
+    // element's role by setting the role attribute.
     AddMenuListChildren();
   } else if (RoleValue() == ax::mojom::blink::Role::kMenuListPopup) {
+    if (RuntimeEnabledFeatures::CustomizableSelectEnabled()) {
+      // Only the UA popover element should have the MenuListPopup mapping.
+      CHECK(HTMLSelectElement::IsPopoverForAppearanceBase(GetNode()));
+    }
     AddMenuListPopupChildren();
   } else if (HasValidHTMLTableStructureAndLayout()) {
     AddTableChildren();
@@ -6099,8 +6109,14 @@ bool AXNodeObject::CanHaveChildren() const {
   // and improving stability in Blink.
   bool result = !GetElement() || AXObject::CanHaveChildren(*GetElement());
   switch (native_role_) {
-    case ax::mojom::blink::Role::kCheckBox:
     case ax::mojom::blink::Role::kListBoxOption:
+      if (RuntimeEnabledFeatures::CustomizableSelectEnabled()) {
+        // When CustomizableSelect is enabled, then options are allowed to have
+        // children as per the new content model.
+        break;
+      }
+      [[fallthrough]];
+    case ax::mojom::blink::Role::kCheckBox:
     case ax::mojom::blink::Role::kMenuItem:
     case ax::mojom::blink::Role::kMenuItemCheckBox:
     case ax::mojom::blink::Role::kMenuItemRadio:

third_party/blink/renderer/modules/accessibility/ax_object.cc

@@ -978,6 +978,30 @@ Node* AXObject::GetParentNodeForComputeParent(AXObjectCacheImpl& cache,
     return node->OwnerShadowHost();
   }
 
+  Node* parent = nullptr;
+
+  // Select elements have a complex architecture with two different slot
+  // elements which `node` may be slotted into. `node` might also not be slotted
+  // into anything at all (which is why we are doing this before using
+  // LayoutTreeBuilderTraversal). Despite this, we always want to expose a
+  // consistent structure for the select element with a MenuList and
+  // MenuListPopup with all the children exposed in the MenuListPopup. For
+  // consistency, we will use the select's PopoverForAppearanceBase element as
+  // the MenuListPopup and make it the parent of all the nodes inside the
+  // select.
+  if (RuntimeEnabledFeatures::CustomizableSelectEnabled()) {
+    if (auto* select = DynamicTo<HTMLSelectElement>(node->parentNode())) {
+      if (select->UsesMenuList()) {
+        if (node == select->SlottedButton()) {
+          // <select>'s author provided <button> should not have any
+          // accessibility mappings.
+          return nullptr;
+        }
+        parent = select->PopoverForAppearanceBase();
+      }
+    }
+  }
+
   // Use LayoutTreeBuilderTraversal::Parent(), which handles pseudo content.
   // This can return nullptr for a node that is never visited by
   // LayoutTreeBuilderTraversal's child traversal. For example, while an element
@@ -988,35 +1012,8 @@ Node* AXObject::GetParentNodeForComputeParent(AXObjectCacheImpl& cache,
   // Whenever null is returned from this function, then a parent cannot be
   // computed, and when a parent is not provided or computed, the accessible
   // object will not be created.
-  Node* parent = LayoutTreeBuilderTraversal::Parent(*node);
+  if (!parent) {
-
+    parent = LayoutTreeBuilderTraversal::Parent(*node);
-  // The parent of a customizable select's popup is the select.
-  if (IsA<HTMLDataListElement>(node)) {
-    if (auto* select = DynamicTo<HTMLSelectElement>(node->OwnerShadowHost())) {
-      if (node == select->PopoverForAppearanceBase()) {
-        return select;
-      }
-    }
-  }
-
-  // For the content of a customizable select, the parent must be the element
-  // assigned the role of kMenuListPopup. To accomplish this, it is necessary to
-  // adapt to unusual DOM structure. If no parent, or the parent has a <select>
-  // shadow host, then the actual parent should be the <select>.
-  // TODO(aleventhal, jarhar): try to simplify this code. @jarhar wrote in code
-  // review: "I don't think that a UA <slot> will ever get returned by
-  // LayoutTreeBuilderTraversal::Parent. In this case, I think
-  // LayoutTreeBuilderTraversal::Parent should just return the <select>."
-
-  HTMLSelectElement* owner_select = nullptr;
-  if (IsA<HTMLSlotElement>(parent) && parent->IsInUserAgentShadowRoot()) {
-    owner_select = DynamicTo<HTMLSelectElement>(parent->OwnerShadowHost());
-  } else if (!parent) {
-    owner_select = DynamicTo<HTMLSelectElement>(NodeTraversal::Parent(*node));
-  }
-  if (owner_select && owner_select->IsAppearanceBasePicker()) {
-    // Return the popup's <datalist> element.
-    return owner_select->PopoverForAppearanceBase();
   }
 
   // No parent: this can occur when elements are not assigned a slot.
@@ -1119,9 +1116,17 @@ bool AXObject::CanHaveChildren(Element& element) {
   // For consistency with the past, options with a single text child are leaves.
   // However, options can now sometimes have interesting children, for
   // a <select> menulist that uses appearance:base-select.
+  // This code looks at IsAppearanceBaseButton instead of IsAppearanceBasePicker
+  // in order to have the same result whether the picker is open or closed.
+  // Ideally we would check to see if the picker is opted in to base appearance,
+  // but without a style update that would change based on whether the picker is
+  // open or not. When the button is opted in then the user will likely be able
+  // to see the child node structure of the option element in the button, so we
+  // can use that as a signal to allow options to have children.
   if (auto* option = DynamicTo<HTMLOptionElement>(element)) {
     return option->OwnerSelectElement() &&
-           option->OwnerSelectElement()->IsAppearanceBasePicker() &&
+           option->OwnerSelectElement()->IsAppearanceBaseButton(
+               HTMLSelectElement::StyleUpdateBehavior::kDontUpdateStyle) &&
            !option->HasOneTextChild();
   }
 
@@ -7337,7 +7342,8 @@ bool AXObject::OnNativeClickAction() {
 
   // Forward default action on custom select to its button.
   if (auto* select = DynamicTo<HTMLSelectElement>(GetNode())) {
-    if (select->IsAppearanceBaseButton()) {
+    if (select->IsAppearanceBaseButton(
+            HTMLSelectElement::StyleUpdateBehavior::kDontUpdateStyle)) {
       if (auto* button = select->SlottedButton()) {
         element = button;
       }

third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc

@@ -165,7 +165,9 @@ Node* RetargetInput(Node* node) {
       possible_select = NodeTraversal::Parent(*node);
     }
     if (auto* select = DynamicTo<HTMLSelectElement>(possible_select)) {
-      if (select->IsAppearanceBaseButton() && node == select->SlottedButton()) {
+      if (select->IsAppearanceBaseButton(
+              HTMLSelectElement::StyleUpdateBehavior::kDontUpdateStyle) &&
+          node == select->SlottedButton()) {
         return select;
       }
     }
@@ -1225,7 +1227,8 @@ bool AXObjectCacheImpl::IsRelevantSlotElement(const HTMLSlotElement& slot) {
   DCHECK(AXObject::CanSafelyUseFlatTreeTraversalNow(slot.GetDocument()));
   DCHECK(slot.SupportsAssignment());
 
-  if (slot.IsInUserAgentShadowRoot() &&
+  if (!RuntimeEnabledFeatures::CustomizableSelectEnabled() &&
+      slot.IsInUserAgentShadowRoot() &&
       IsA<HTMLSelectElement>(slot.OwnerShadowHost())) {
     return slot.GetIdAttribute() == shadow_element_names::kSelectOptions;
   }

third_party/blink/web_tests/fast/forms/select/customizable-select/appearance-base-select-usecounter.html

@@ -3,6 +3,8 @@
 <link rel=help href="https://issues.chromium.org/issues/361129480">
 <script src="../../../../resources/testharness.js"></script>
 <script src="../../../../resources/testharnessreport.js"></script>
+<script src="../../../../resources/testdriver.js"></script>
+<script src="../../../../resources/testdriver-vendor.js"></script>
 
 <select>
   <option>option</option>
@@ -49,7 +51,13 @@ promise_test(async () => {
   await new Promise(requestAnimationFrame);
   assert_true(internals.isUseCounted(document, buttonUseCounterId),
     'Button use counter should be hit with base-select on button and picker.');
+  assert_false(internals.isUseCounted(document, pickerUseCounterId),
+    "Picker use counter should not be hit if picker isn't open.");
+
+  await test_driver.click(select);
+  await new Promise(requestAnimationFrame);
+  await new Promise(requestAnimationFrame);
   assert_true(internals.isUseCounted(document, pickerUseCounterId),
-    'Picker use counter should be hit with base-select on button and picker.');
+    'Picker use counter should be hit after picker is opened.');
 }, 'UseCounters for base-select on button and picker should work.');
 </script>

third_party/blink/web_tests/http/tests/inspector-protocol/accessibility/accessibility-ignoredNodes-expected.txt

@@ -423,7 +423,6 @@ RootWebArea
     none
       *combobox
         MenuListPopup
-        MenuListPopup
 {
     childIds : <object>
     chromeRole : {

third_party/blink/web_tests/virtual/popover-anchor-relationships-disabled/external/wpt/html/semantics/forms/the-select-element/customizable-select/switch-picker-appearance.tentative-expected.txt

@@ -0,0 +1,17 @@
+This is a testharness.js-based test.
+[FAIL] Basic functionality of select picker with appearance:auto
+  assert_equals: expected "none" but got "base-select"
+[FAIL] Basic functionality of select picker with appearance:none
+  assert_equals: expected "none" but got "base-select"
+[FAIL] Switching appearance in popover-open should close the picker
+  assert_equals: expected "none" but got "base-select"
+[FAIL] Switching appearance in JS after picker is open should close the picker
+  assert_equals: expected "none" but got "base-select"
+[FAIL] The select picker is closed if the <select> appearance value is changed via CSS while the picker is open
+  assert_false: picker should get closed when the appearance value changes expected false got true
+[FAIL] The select picker is closed if the ::picker() appearance value is changed via CSS while the picker is open
+  assert_false: setup expected false got true
+[FAIL] The select picker is closed if the <select> inline appearance value is changed while the picker is open
+  assert_false: setup expected false got true
+Harness: the test ran to completion.
+