Skip to content

Bug 2070311 - Fix interaction of rule cache reset reuse and visited optimization. r=#style - #360

Closed
emilio wants to merge 1 commit into
mozilla-firefox:autolandfrom
emilio:visited-cached-reset-properties
Closed

emilio wants to merge 1 commit into
mozilla-firefox:autolandfrom
emilio:visited-cached-reset-properties

Conversation

@emilio

@emilio emilio commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

We're hitting the "used cached reset properties" path, and thus we don't
notice that .item indeed sets border-color, which is a visited dependent
property. So we end up incorrectly with border-color: currentcolor in
the computed style, and thus wrong rendering.

Track the "used visited dependent properties" with a style bit and carry
that around instead.


Lando: link
Bugzilla: bug 2070311

⚠️ This pull request has 3 warnings.
🚫 This pull request has 1 blocker.

…ptimization. r=#style

We're hitting the "used cached reset properties" path, and thus we don't
notice that .item indeed sets border-color, which is a visited dependent
property. So we end up incorrectly with `border-color: currentcolor` in
the computed style, and thus wrong rendering.

Track the "used visited dependent properties" with a style bit and carry
that around instead.
@lando-web
lando-web Bot requested a review from a team September 9, 2026 11:13
@emilio
emilio requested a review from a team September 9, 2026 11:13
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

View this pull request in Lando to land it once approved.

);

using_cached_reset_properties =
cascade.try_to_use_cached_reset_properties(&mut context, rule_cache, guards);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using cached reset props here

cascade.try_to_use_cached_reset_properties(&mut context, rule_cache, guards);

if using_cached_reset_properties {
LonghandIdSet::late_group_only_inherited()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

border properties aren't inherited, so excluded from here

// visited-dependent properties, we can avoid gathering the declarations, we know none
// would be relevant.
if unvisited_context.builder.rules.as_ref() != Some(rules)
|| unvisited_properties.contains_any(visited_dependent_props)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

border-color never gets seen here, so we skip over this

@mozilla-code-review

Copy link
Copy Markdown

No new issues detected. This pull request is 🆗

@lando-worker

lando-worker Bot commented Sep 9, 2026

Copy link
Copy Markdown

Pull request closed by commit 8c2245f

lando-worker Bot pushed a commit that referenced this pull request Sep 9, 2026
…ptimization. r=dshin

We're hitting the "used cached reset properties" path, and thus we don't
notice that .item indeed sets border-color, which is a visited dependent
property. So we end up incorrectly with `border-color: currentcolor` in
the computed style, and thus wrong rendering.

Track the "used visited dependent properties" with a style bit and carry
that around instead.

Pull request: #360
@lando-worker lando-worker Bot closed this Sep 9, 2026
github-actions Bot pushed a commit to DioxusLabs/stylo that referenced this pull request Sep 10, 2026
…ptimization. r=dshin

We're hitting the "used cached reset properties" path, and thus we don't
notice that .item indeed sets border-color, which is a visited dependent
property. So we end up incorrectly with `border-color: currentcolor` in
the computed style, and thus wrong rendering.

Track the "used visited dependent properties" with a style bit and carry
that around instead.

Pull request: mozilla-firefox/firefox#360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants