Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stylo: visited pseudo-class support #17032

Merged
merged 6 commits into from May 25, 2017
Merged

Stylo: visited pseudo-class support #17032

merged 6 commits into from May 25, 2017

Conversation

@jryans
Copy link
Contributor

jryans commented May 24, 2017

jryans added 6 commits May 15, 2017
Adjust the matching process to look for a "relevant link" while matching.  A
"relevant link" is the element being matched if it is a link or the nearest
ancestor link.

Matching for links now depends on the `VisitedHandlingMode`, which determines
whether all links match as if they are unvisited (the default) or if the
relevant link matches as visited (and all others remain unvisited).

If a relevant link is ever found for any selector, track this as part of the
`MatchingContext` object.  This is used in the next patch to determine if an
additional match and cascade should be performed to compute the styles when
visited.

MozReview-Commit-ID: 3xUbRo7vpuD
To support visited styles, we match and cascade a separate set of styles any
time we notice that an element has a relevant link.

The visited rules and values are held in `ComputedStyle` alongside the
regular rules and values, which simplifies supporting various APIs like
`cascade_primary_and_pseudos` which expect easy access to previously matched
rules.

To simplify passing the additional values around, an additional reference to the
visited `ComputedValues` is placed inside the regular `ComputedValues`.

MozReview-Commit-ID: 2ebbjcfkfWf
Since visited rules are stored separately, we need also run `replace_rules` for
those in addition to regular rules.

MozReview-Commit-ID: 4KYhOBXm88O
Extend restyle hints to check additional cases for visited.

If we are sensitive to visitedness and the visited state changed, we force a
restyle here.  Matching doesn't depend on the actual visited state at all, so we
can't look at matching results to decide what to do for this case.

This also updates the snapshot version of `match_non_ts_pseudo_class` to check
the relevant link instead of the element state, just like we do when matching
with regular Gecko elements.

There's also a separate case of a visited-dependent selector when other state or
attributes change.  For this, we need to cover both types of matching we use
when cascading values.  If there is a relevant link, then we also matched in
visited mode.  Match again in this mode to ensure this also matches.  Note that
we never actually match directly against the element's true visited state at
all, since that would expose us to timing attacks.  The matching process only
considers the relevant link state and visited handling mode when deciding if
visited matches.

MozReview-Commit-ID: CVGquetQ2VW
The style sharing cache stores the regular `ComputedValues`, so it would also
have the visited values as well for anything inserted into the cache (since they
are nested inside).  Unlike all other element states, a change in state of
unvisited vs. visited does not change the style system's output, since we have
already computed both possible outputs up front.

We change the element state checks when looking for style sharing cache hits to
ignore visitedness, since that's handled by the two separate sets of values.

MozReview-Commit-ID: Dt8uK8gSQSP
Speed up the visited cascade by only running it for the properties that are
actually visited dependent.  (These are only the properties where the separate
set of visited styles is even read at all, so running the rest is wasted work.)

MozReview-Commit-ID: 5B7wYtuH974
@highfive
Copy link

highfive commented May 24, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/gecko.mako.rs, components/style/data.rs, components/style/sharing/mod.rs, components/style/properties/properties.mako.rs, components/style/restyle_hints.rs and 8 more
  • @KiChjang: components/script/dom/element.rs, components/script/layout_wrapper.rs
  • @fitzgen: components/script/dom/element.rs, components/script/layout_wrapper.rs
  • @emilio: components/style/properties/gecko.mako.rs, ports/geckolib/glue.rs, components/style/data.rs, components/style/sharing/mod.rs, components/style/properties/properties.mako.rs and 9 more
@highfive
Copy link

highfive commented May 24, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style and script code, but no tests are modified. Please consider adding a test!
@jryans
Copy link
Contributor Author

jryans commented May 24, 2017

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

bors-servo commented May 24, 2017

📌 Commit f12af6c has been approved by emilio

@highfive highfive assigned emilio and unassigned jdm May 24, 2017
@jryans
Copy link
Contributor Author

jryans commented May 24, 2017

@bors-servo
Copy link
Contributor

bors-servo commented May 24, 2017

Testing commit f12af6c with merge 525790f...

bors-servo added a commit that referenced this pull request May 24, 2017
Stylo: visited pseudo-class support

Reviewed in https://bugzilla.mozilla.org/show_bug.cgi?id=1328509

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17032)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 24, 2017

💔 Test failed - linux-dev

@jryans
Copy link
Contributor Author

jryans commented May 24, 2017

@bors-servo retry

@jryans
Copy link
Contributor Author

jryans commented May 24, 2017

The last run hit the intermittent #16783 issue, hopefully the next run will work.

@jryans
Copy link
Contributor Author

jryans commented May 25, 2017

@bors-servo
Copy link
Contributor

bors-servo commented May 25, 2017

Testing commit f12af6c with merge 1f323f8...

bors-servo added a commit that referenced this pull request May 25, 2017
Stylo: visited pseudo-class support

Reviewed in https://bugzilla.mozilla.org/show_bug.cgi?id=1328509

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17032)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 25, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: emilio
Pushing 1f323f8 to master...

@bors-servo bors-servo mentioned this pull request May 25, 2017
4 of 5 tasks complete
@bors-servo bors-servo merged commit f12af6c into servo:master May 25, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.