selectors: Simplify :visited by only using the "is inside link" information. #19520
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mation. Right now we go through a lot of hoops to see if we ever see a relevant link. However, that information is not needed: if the element is a link, we'll always need to compute its visited style because its its own relevant link. If the element inherits from a link, we need to also compute the visited style anyway. So the "has a relevant link been found" is pretty useless when we know what are we inheriting from. The branches at the beginning of matches_complex_selector_internal were affecting performance, and there are no good reasons to keep them. I've verified that this passes all the visited tests in mozilla central, and that the test-cases too-flaky to be landed still pass.
|
Heads up! This PR modifies the following files:
|
|
|
|
r? @jryans |
jryans
approved these changes
Dec 8, 2017
|
Yay, thanks for the review @jryans! @bors-servo r=jryans p=1 |
|
📌 Commit 3119db7 has been approved by |
highfive
added
S-awaiting-merge
bors-servo
pushed a commit
that referenced
this pull request
Dec 8, 2017
selectors: Simplify :visited by only using the "is inside link" information. Right now we go through a lot of hoops to see if we ever see a relevant link. However, that information is not needed: if the element is a link, we'll always need to compute its visited style because its its own relevant link. If the element inherits from a link, we need to also compute the visited style anyway. So the "has a relevant link been found" is pretty useless when we know what are we inheriting from. The branches at the beginning of matches_complex_selector_internal were affecting performance, and there are no good reasons to keep them. I've verified that this passes all the visited tests in mozilla central, and that the test-cases too-flaky to be landed still pass. <!-- 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/19520) <!-- Reviewable:end -->
|
💔 Test failed - android |
highfive
added
S-tests-failed
|
@bors-servo retry
|
bors-servo
pushed a commit
that referenced
this pull request
Dec 8, 2017
selectors: Simplify :visited by only using the "is inside link" information. Right now we go through a lot of hoops to see if we ever see a relevant link. However, that information is not needed: if the element is a link, we'll always need to compute its visited style because its its own relevant link. If the element inherits from a link, we need to also compute the visited style anyway. So the "has a relevant link been found" is pretty useless when we know what are we inheriting from. The branches at the beginning of matches_complex_selector_internal were affecting performance, and there are no good reasons to keep them. I've verified that this passes all the visited tests in mozilla central, and that the test-cases too-flaky to be landed still pass. <!-- 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/19520) <!-- Reviewable:end -->
highfive
added
S-awaiting-merge
|
💔 Test failed - mac-dev-unit |
highfive
added
S-tests-failed
|
@bors-servo retry
|
bors-servo
pushed a commit
that referenced
this pull request
Dec 8, 2017
selectors: Simplify :visited by only using the "is inside link" information. Right now we go through a lot of hoops to see if we ever see a relevant link. However, that information is not needed: if the element is a link, we'll always need to compute its visited style because its its own relevant link. If the element inherits from a link, we need to also compute the visited style anyway. So the "has a relevant link been found" is pretty useless when we know what are we inheriting from. The branches at the beginning of matches_complex_selector_internal were affecting performance, and there are no good reasons to keep them. I've verified that this passes all the visited tests in mozilla central, and that the test-cases too-flaky to be landed still pass. <!-- 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/19520) <!-- Reviewable:end -->
highfive
added
S-awaiting-merge
|
☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev |
highfive
removed
the
S-awaiting-merge
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Aug 15, 2018
We match with AllLinksVisitedAndUnvisited for style invalidation, and we already do a subtree restyle because :visited matching doesn't depend on the actual element state. So all this stuff is just not needed. The comment points to the attribute tests in bug 1328509, but those still trivially pass with this change. I think this was unneeded since I introduced AllLinksVisitedAndUnvisited, or maybe since servo/servo#19520. In any case it doesn't really matter, and I already had done this cleanup in my WIP patches for bug 1406622, but I guess this is a slightly more suitable place to land them :) Differential Revision: https://phabricator.services.mozilla.com/D3305
jankeromnes
pushed a commit
to jankeromnes/gecko
that referenced
this pull request
Aug 15, 2018
We match with AllLinksVisitedAndUnvisited for style invalidation, and we already do a subtree restyle because :visited matching doesn't depend on the actual element state. So all this stuff is just not needed. The comment points to the attribute tests in bug 1328509, but those still trivially pass with this change. I think this was unneeded since I introduced AllLinksVisitedAndUnvisited, or maybe since servo/servo#19520. In any case it doesn't really matter, and I already had done this cleanup in my WIP patches for bug 1406622, but I guess this is a slightly more suitable place to land them :) Differential Revision: https://phabricator.services.mozilla.com/D3305
emilio
added a commit
to emilio/servo
that referenced
this pull request
Aug 18, 2018
We match with AllLinksVisitedAndUnvisited for style invalidation, and we already do a subtree restyle because :visited matching doesn't depend on the actual element state. So all this stuff is just not needed. The comment points to the attribute tests in bug 1328509, but those still trivially pass with this change. I think this was unneeded since I introduced AllLinksVisitedAndUnvisited, or maybe since servo#19520. In any case it doesn't really matter, and I already had done this cleanup in my WIP patches for bug 1406622, but I guess this is a slightly more suitable place to land them :) Differential Revision: https://phabricator.services.mozilla.com/D3305
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Oct 3, 2019
We match with AllLinksVisitedAndUnvisited for style invalidation, and we already do a subtree restyle because :visited matching doesn't depend on the actual element state. So all this stuff is just not needed. The comment points to the attribute tests in bug 1328509, but those still trivially pass with this change. I think this was unneeded since I introduced AllLinksVisitedAndUnvisited, or maybe since servo/servo#19520. In any case it doesn't really matter, and I already had done this cleanup in my WIP patches for bug 1406622, but I guess this is a slightly more suitable place to land them :) Differential Revision: https://phabricator.services.mozilla.com/D3305 UltraBlame original commit: 4c42d90e56a6e6364ca0ed9de11aaa79ea2f4706
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Oct 3, 2019
We match with AllLinksVisitedAndUnvisited for style invalidation, and we already do a subtree restyle because :visited matching doesn't depend on the actual element state. So all this stuff is just not needed. The comment points to the attribute tests in bug 1328509, but those still trivially pass with this change. I think this was unneeded since I introduced AllLinksVisitedAndUnvisited, or maybe since servo/servo#19520. In any case it doesn't really matter, and I already had done this cleanup in my WIP patches for bug 1406622, but I guess this is a slightly more suitable place to land them :) Differential Revision: https://phabricator.services.mozilla.com/D3305 UltraBlame original commit: 4c42d90e56a6e6364ca0ed9de11aaa79ea2f4706
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Oct 3, 2019
We match with AllLinksVisitedAndUnvisited for style invalidation, and we already do a subtree restyle because :visited matching doesn't depend on the actual element state. So all this stuff is just not needed. The comment points to the attribute tests in bug 1328509, but those still trivially pass with this change. I think this was unneeded since I introduced AllLinksVisitedAndUnvisited, or maybe since servo/servo#19520. In any case it doesn't really matter, and I already had done this cleanup in my WIP patches for bug 1406622, but I guess this is a slightly more suitable place to land them :) Differential Revision: https://phabricator.services.mozilla.com/D3305 UltraBlame original commit: 4c42d90e56a6e6364ca0ed9de11aaa79ea2f4706
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now we go through a lot of hoops to see if we ever see a relevant link.
However, that information is not needed: if the element is a link, we'll always
need to compute its visited style because its its own relevant link.
If the element inherits from a link, we need to also compute the visited style
anyway.
So the "has a relevant link been found" is pretty useless when we know what are
we inheriting from.
The branches at the beginning of matches_complex_selector_internal were
affecting performance, and there are no good reasons to keep them.
I've verified that this passes all the visited tests in mozilla central, and
that the test-cases too-flaky to be landed still pass.
This change is