Closed Bug 1381635 Opened 7 years ago Closed 7 years ago

stylo: Fix sharing of visited style contexts

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: manishearth, Assigned: manishearth)

References

Details

Attachments

(1 file)

In bug 1367904 we start using the servo-side visited styles.

However, if styles are being shared, this may mark unvisited styles being shared (e.g. siblings) as visited too.

We were failing some mochitests because of this and marked them as expected.



FAIL | layout/style/test/test_visited_reftests.html | reftest comparison: == selector-descendant-2.xhtml selector-descendant-2-ref.xhtml [log…]

 
FAIL | layout/style/test/test_visited_reftests.html | reftest comparison: == subject-of-selector-descendant-2.xhtml subject-of-selector-descendant-2-ref.xhtml [log…]



We should either Arc::make_mut these when setting visited styles, or:

14:23 < emilio> bholley: if so, fine for me... You can also just change the same_state_ignoring_visitedness to not ignore visitedness,
                I guess
Not ignoring visitedness seems like the right solution to me, fwiw.
Attachment #8887594 - Flags: review?(bzbarsky)
Comment on attachment 8887594 [details]
Bug 1381635 - stylo: Don't ignore visited state when deciding to share style contexts;

https://reviewboard.mozilla.org/r/158460/#review163710

::: servo/components/style/sharing/mod.rs:646
(Diff revision 2)
>          if target.matches_user_and_author_rules() !=
>              candidate.element.matches_user_and_author_rules() {
>              miss!(UserAndAuthorRules)
>          }
>  
> -        if !checks::have_same_state_ignoring_visitedness(target.element, candidate) {
> +        if target.element.get_state() != candidate.get_state() {

This should have a comment about why we're including visitedness state, because it's definitely not obvious.
Attachment #8887594 - Flags: review?(bzbarsky) → review+
Comment on attachment 8887594 [details]
Bug 1381635 - stylo: Don't ignore visited state when deciding to share style contexts;

https://reviewboard.mozilla.org/r/158460/#review163760

::: layout/style/test/stylo-failures.md:70
(Diff revision 3)
>  * test_author_specified_style.html: support serializing color as author specified bug 1348165 [27]
>  * browser_newtab_share_rule_processors.js: agent style sheet sharing [1]
>  * :visited support (bug 1328509)
> -  * test_visited_reftests.html `inherit-keyword-1.xhtml` [2]
> -  * test_visited_reftests.html `selector-descendant-2.xhtml`: bug 1381635 [4]
>    * ... `mathml-links.html` [2]

This line should now list the visited test file, instead of just "...".
Maybe this fixes the mathml-links.html one as well?
IIRC it doesn't, the mathml failure was always there
https://github.com/servo/servo/pull/17778



https://hg.mozilla.org/mozilla-central/rev/d396287f2595
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: