From a0e29d70328a3d29b4c9abd90fc7ad9276004ab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 31 May 2023 11:37:50 +0200 Subject: [PATCH] style: Refactor :-moz-lwtheme pseudo-classes to get invalidated correctly Use the same document state mechanism we have for :moz-locale-dir. Also, simplify the setup of the later to be the same as :dir(), allowing the matching code to be less repetitive. This should fix some flakiness in chrome mochitests, but we have no existing tests for these pseudo-classes more generally and since they're just chrome-only I'm not super-excited about adding more. Differential Revision: https://phabricator.services.mozilla.com/D130735 --- components/style/element_state.rs | 14 +++++-- components/style/gecko/selector_parser.rs | 15 ++++++-- components/style/gecko/wrapper.rs | 46 ++++++----------------- 3 files changed, 35 insertions(+), 40 deletions(-) diff --git a/components/style/element_state.rs b/components/style/element_state.rs index 8a9f4065b8b..774e3936d2a 100644 --- a/components/style/element_state.rs +++ b/components/style/element_state.rs @@ -137,9 +137,17 @@ bitflags! { /// dom/base/Document.h. #[derive(MallocSizeOf)] pub struct DocumentState: u64 { - /// RTL locale: specific to the XUL localedir attribute - const NS_DOCUMENT_STATE_RTL_LOCALE = 1 << 0; /// Window activation status - const NS_DOCUMENT_STATE_WINDOW_INACTIVE = 1 << 1; + const WINDOW_INACTIVE = 1 << 0; + /// RTL locale: specific to the XUL localedir attribute + const RTL_LOCALE = 1 << 1; + /// LTR locale: specific to the XUL localedir attribute + const LTR_LOCALE = 1 << 2; + /// LWTheme status + const LWTHEME = 1 << 3; + /// LWTheme status + const LWTHEME_BRIGHTTEXT = 1 << 4; + /// LWTheme status + const LWTHEME_DARKTEXT = 1 << 5; } } diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index e9d2c28ccfb..751a38d8465 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -8,7 +8,7 @@ use crate::element_state::{DocumentState, ElementState}; use crate::gecko_bindings::structs::RawServoSelectorList; use crate::gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI}; use crate::invalidation::element::document_state::InvalidationMatchingData; -use crate::selector_parser::{Direction, SelectorParser}; +use crate::selector_parser::{Direction, HorizontalDirection, SelectorParser}; use crate::str::starts_with_ignore_ascii_case; use crate::string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; use crate::values::{AtomIdent, AtomString}; @@ -174,8 +174,17 @@ impl NonTSPseudoClass { /// Get the document state flag associated with a pseudo-class, if any. pub fn document_state_flag(&self) -> DocumentState { match *self { - NonTSPseudoClass::MozLocaleDir(..) => DocumentState::NS_DOCUMENT_STATE_RTL_LOCALE, - NonTSPseudoClass::MozWindowInactive => DocumentState::NS_DOCUMENT_STATE_WINDOW_INACTIVE, + NonTSPseudoClass::MozLocaleDir(ref dir) => { + match dir.as_horizontal_direction() { + Some(HorizontalDirection::Ltr) => DocumentState::LTR_LOCALE, + Some(HorizontalDirection::Rtl) => DocumentState::RTL_LOCALE, + None => DocumentState::empty(), + } + }, + NonTSPseudoClass::MozWindowInactive => DocumentState::WINDOW_INACTIVE, + NonTSPseudoClass::MozLWTheme => DocumentState::LWTHEME, + NonTSPseudoClass::MozLWThemeBrightText => DocumentState::LWTHEME_BRIGHTTEXT, + NonTSPseudoClass::MozLWThemeDarkText => DocumentState::LWTHEME_DARKTEXT, _ => DocumentState::empty(), } } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 09dbc6f06c1..d2a86caf100 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -39,11 +39,10 @@ use crate::gecko_bindings::bindings::Gecko_IsSignificantChild; use crate::gecko_bindings::bindings::Gecko_MatchLang; use crate::gecko_bindings::bindings::Gecko_UnsetDirtyStyleAttr; use crate::gecko_bindings::bindings::Gecko_UpdateAnimations; -use crate::gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetDocumentLWTheme}; +use crate::gecko_bindings::bindings::Gecko_ElementState; use crate::gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags}; use crate::gecko_bindings::structs; use crate::gecko_bindings::structs::nsChangeHint; -use crate::gecko_bindings::structs::Document_DocumentTheme as DocumentTheme; use crate::gecko_bindings::structs::EffectCompositor_CascadeLevel as CascadeLevel; use crate::gecko_bindings::structs::ELEMENT_HANDLED_SNAPSHOT; use crate::gecko_bindings::structs::ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO; @@ -62,7 +61,7 @@ use crate::properties::animated_properties::{AnimationValue, AnimationValueMap}; use crate::properties::{ComputedValues, LonghandId}; use crate::properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; use crate::rule_tree::CascadeLevel as ServoCascadeLevel; -use crate::selector_parser::{AttrValue, HorizontalDirection, Lang}; +use crate::selector_parser::{AttrValue, Lang}; use crate::shared_lock::{Locked, SharedRwLock}; use crate::string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; use crate::stylist::CascadeData; @@ -752,12 +751,6 @@ impl<'le> GeckoElement<'le> { .get_bool_flag(nsINode_BooleanFlag::ElementMayHaveStyle) } - #[inline] - fn document_theme(&self) -> DocumentTheme { - let node = self.as_node(); - unsafe { Gecko_GetDocumentLWTheme(node.owner_doc().0) } - } - /// Only safe to call on the main thread, with exclusive access to the /// element and its ancestors. /// @@ -2033,39 +2026,24 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { bindings::Gecko_IsSelectListBox(self.0) }, NonTSPseudoClass::MozIsHTML => self.is_html_element_in_html_document(), - NonTSPseudoClass::MozLWTheme => self.document_theme() != DocumentTheme::Doc_Theme_None, - NonTSPseudoClass::MozLWThemeBrightText => { - self.document_theme() == DocumentTheme::Doc_Theme_Bright - }, - NonTSPseudoClass::MozLWThemeDarkText => { - self.document_theme() == DocumentTheme::Doc_Theme_Dark - }, + + NonTSPseudoClass::MozLWTheme | + NonTSPseudoClass::MozLWThemeBrightText | + NonTSPseudoClass::MozLWThemeDarkText | + NonTSPseudoClass::MozLocaleDir(..) | NonTSPseudoClass::MozWindowInactive => { - let state_bit = DocumentState::NS_DOCUMENT_STATE_WINDOW_INACTIVE; + let state_bit = pseudo_class.document_state_flag(); + if state_bit.is_empty() { + debug_assert!(matches!(pseudo_class, NonTSPseudoClass::MozLocaleDir(..)), "Only moz-locale-dir should ever return an empty state"); + return false; + } if context.extra_data.document_state.intersects(state_bit) { return !context.in_negation(); } - self.document_state().contains(state_bit) }, NonTSPseudoClass::MozPlaceholder => false, NonTSPseudoClass::Lang(ref lang_arg) => self.match_element_lang(None, lang_arg), - NonTSPseudoClass::MozLocaleDir(ref dir) => { - let state_bit = DocumentState::NS_DOCUMENT_STATE_RTL_LOCALE; - if context.extra_data.document_state.intersects(state_bit) { - // NOTE(emilio): We could still return false for values - // other than "ltr" and "rtl", but we don't bother. - return !context.in_negation(); - } - - let doc_is_rtl = self.document_state().contains(state_bit); - - match dir.as_horizontal_direction() { - Some(HorizontalDirection::Ltr) => !doc_is_rtl, - Some(HorizontalDirection::Rtl) => doc_is_rtl, - None => false, - } - }, } }