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 <mrobinson@igalia.com>
This commit is contained in:
Martin Robinson 2025-09-03 01:50:01 -07:00 committed by GitHub
parent 4dcf4ef07a
commit 6b208124dc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 20 additions and 20 deletions

View file

@ -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) { pub(crate) fn element_attr_will_change(&self, el: &Element, attr: &Attr) {
// FIXME(emilio): Kind of a shame we have to duplicate this. // FIXME(emilio): Kind of a shame we have to duplicate this.
// //

View file

@ -49,7 +49,7 @@ use style::properties::{
}; };
use style::rule_tree::CascadeLevel; use style::rule_tree::CascadeLevel;
use style::selector_parser::{ use style::selector_parser::{
NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser, NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser, Snapshot,
extended_filtering, extended_filtering,
}; };
use style::shared_lock::Locked; use style::shared_lock::Locked;
@ -5403,8 +5403,23 @@ impl Element {
return; return;
} }
let node = self.upcast::<Node>(); // Add a pending restyle for this node which captures a snapshot of the state
node.owner_doc().element_state_will_change(self); // 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::<Node>().dirty(NodeDamage::ContentOrHeritage);
self.state.set(state); self.state.set(state);
} }
@ -5422,7 +5437,6 @@ impl Element {
} }
pub(crate) fn set_focus_state(&self, value: bool) { pub(crate) fn set_focus_state(&self, value: bool) {
self.upcast::<Node>().dirty(NodeDamage::Other);
self.set_state(ElementState::FOCUS, value); self.set_state(ElementState::FOCUS, value);
} }
@ -5431,7 +5445,7 @@ impl Element {
} }
pub(crate) fn set_hover_state(&self, value: bool) { 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 { pub(crate) fn enabled_state(&self) -> bool {
@ -5463,10 +5477,7 @@ impl Element {
} }
pub(crate) fn set_placeholder_shown_state(&self, value: bool) { pub(crate) fn set_placeholder_shown_state(&self, value: bool) {
if self.placeholder_shown_state() != value { self.set_state(ElementState::PLACEHOLDER_SHOWN, value);
self.set_state(ElementState::PLACEHOLDER_SHOWN, value);
self.upcast::<Node>().dirty(NodeDamage::Other);
}
} }
pub(crate) fn set_target_state(&self, value: bool) { pub(crate) fn set_target_state(&self, value: bool) {