Bug 325152

Created attachment 481541 [details] test Rebuilding a select's renderers while its base appearance picker is open leaves the picker on screen but stops :open from matching. The picker keeps rendering its options and keeps keyboard focus, so any styling or scripting keyed on :open is wrong for as long as the picker stays up. Steps, with the attached test or by hand: 1. <select style="appearance: base-select"> with ::picker(select) { appearance: base-select }. 2. Click it to open the picker. 3. From the console, select.size = 1. Measured before and after step 3: opened: :open=true option rendered=true activeElement=option after rebuild: :open=false option rendered=true activeElement=option Setting size from absent to 1 leaves it a dropdown box but calls invalidateStyleAndRenderersForSubtree(), and didDetachRenderers() (HTMLSelectElement.cpp:168) then clears m_popupIsVisible unconditionally. That is correct for the native popup menu it tears down in the same breath, but a base appearance picker is a popover whose showing state is independent of the select's renderers, so it survives the rebuild while the flag does not. The underlying problem is that :open is a single stored flag, bool HTMLSelectElement::isOpen() const { return m_popupIsVisible; } serving two owners with different lifetimes: the native popup menu, which dies with the renderers, and the picker popover, which does not. Anything that tears renderers down clobbers state belonging to the other owner. Patching it by clearing on detach and restoring on attach works but leaves two hooks agreeing about a fact neither owns. The fix that removes the class of bug is to stop storing it and derive it, so there is no moment when the flag can be wrong: bool HTMLSelectElement::isOpen() const { if (RefPtr popover = m_popover; popover && /* base picker */) return popover->isPopoverShowing(); return m_popupIsVisible; } Two things make that a real change rather than a tweak. :open invalidation currently happens inside setPopupIsVisible(), so the popover's show and hide paths have to trigger it instead. And isOpen() runs during selector matching, so the base picker test has to be cheaper than usesBaseAppearancePicker(), which reads two computed styles. The flag has 13 reads and 9 writes. The accessibility readers (AccessibilityMenuList, AccessibilityRenderObject, AccessibilityObject) all want the native meaning and would keep using it as-is; only isOpen() and SelectPopoverElement's check want the picker meaning. This is not iOS-specific but also not reachable there: didDetachRenderers()'s body is entirely #if !PLATFORM(IOS_FAMILY), since m_popup does not exist on iOS. The attached test fails on trunk at the third assertion — select.matches(':open') after the rebuild — while the two beside it, that the option still has a box and still has focus, pass and document that the picker really is showing.