From 6b208124dcaa8a49d4fc5541fef91d1f7aab0ea0 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 3 Sep 2025 01:50:01 -0700 Subject: [PATCH] script: Always dirty nodes when changing node state (#39102) With incremental layout adding a restyle to a node isn't enough to force its layout to update. We also need to explicitly mark the node as dirty so that its contents are updated when layout is run. This change makes this consistent for all node state changes. This might be a bit too conservative as all node state may not affect layout, but should catch issues in the future. Testing: This is very hard to test as it requires moving the mouse over the WebView, and the moving it away, and then testing the rendered contents. This kind of coordination would be difficult to manage with unit tests. Fixes: #38989. Signed-off-by: Martin Robinson --- components/script/dom/document.rs | 11 ----------- components/script/dom/element.rs | 29 ++++++++++++++++++++--------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 0b30786b870..901e2a74254 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -3773,17 +3773,6 @@ impl Document { }) } - pub(crate) fn element_state_will_change(&self, element: &Element) { - let mut entry = self.ensure_pending_restyle(element); - if entry.snapshot.is_none() { - entry.snapshot = Some(Snapshot::new()); - } - let snapshot = entry.snapshot.as_mut().unwrap(); - if snapshot.state.is_none() { - snapshot.state = Some(element.state()); - } - } - pub(crate) fn element_attr_will_change(&self, el: &Element, attr: &Attr) { // FIXME(emilio): Kind of a shame we have to duplicate this. // diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index abd6f4d0290..139b3228193 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -49,7 +49,7 @@ use style::properties::{ }; use style::rule_tree::CascadeLevel; use style::selector_parser::{ - NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser, + NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser, Snapshot, extended_filtering, }; use style::shared_lock::Locked; @@ -5403,8 +5403,23 @@ impl Element { return; } - let node = self.upcast::(); - node.owner_doc().element_state_will_change(self); + // Add a pending restyle for this node which captures a snapshot of the state + // before the change. + { + let document = self.owner_document(); + let mut entry = document.ensure_pending_restyle(self); + if entry.snapshot.is_none() { + entry.snapshot = Some(Snapshot::new()); + } + let snapshot = entry.snapshot.as_mut().unwrap(); + if snapshot.state.is_none() { + snapshot.state = Some(self.state()); + } + } + + // Dirty the node so that it is laid out again if necessary. + self.upcast::().dirty(NodeDamage::ContentOrHeritage); + self.state.set(state); } @@ -5422,7 +5437,6 @@ impl Element { } pub(crate) fn set_focus_state(&self, value: bool) { - self.upcast::().dirty(NodeDamage::Other); self.set_state(ElementState::FOCUS, value); } @@ -5431,7 +5445,7 @@ impl Element { } pub(crate) fn set_hover_state(&self, value: bool) { - self.set_state(ElementState::HOVER, value) + self.set_state(ElementState::HOVER, value); } pub(crate) fn enabled_state(&self) -> bool { @@ -5463,10 +5477,7 @@ impl Element { } pub(crate) fn set_placeholder_shown_state(&self, value: bool) { - if self.placeholder_shown_state() != value { - self.set_state(ElementState::PLACEHOLDER_SHOWN, value); - self.upcast::().dirty(NodeDamage::Other); - } + self.set_state(ElementState::PLACEHOLDER_SHOWN, value); } pub(crate) fn set_target_state(&self, value: bool) {