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

Jump to bottom

Update <link> pseudo selector WPTs #27178

Merged
merged 1 commit into from Jan 19, 2021
Merged

Update <link> pseudo selector WPTs #27178

merged 1 commit into from Jan 19, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jan 14, 2021 •
edited

In these spec PRs [1][2], the <link> element will no longer match :link,
:visited, or :any-link, and it will no longer navigate the page when
clicked.

This patch also modifies the TestExpectations for one of the modified
tests navigation.sub.html. There was one test case which was
consistently timing out which I removed, which would stop the test from
timing out. However, there are an additional two test cases which time
out when run with run_web_tests.py but pass just fine with
run_wpt_tests.py. In addition, the flags passed to run_web_tests.py on
the trybots make it so that those two test cases which time out get
written to a failure file instead of making the whole test runner time
out, which would count as a failure in TestExpectations. Since the
default flags for run_web_tests.py make it an actual timeout according to
TestExpectations, I left the timeout expectation in there as well.

Hopefully this test can just be run with run_wpt_tests.py in the future
and we can get rid of this TestExpectation.

[1] whatwg/html#6269
[2] w3c/csswg-drafts#5839

Bug: 611093
Change-Id: I7234eb6024450e28c1ca6ec1ede7fdfc8f2ece52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628037
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844921}

wpt-pr-bot
wpt-pr-bot approved these changes Jan 14, 2021
View reviewed changes
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

@josepharhar josepharhar mentioned this pull request Jan 14, 2021
Merged
3 tasks
domenic
domenic requested changes Jan 14, 2021
View reviewed changes
html/semantics/selectors/pseudo-classes/utils.js Outdated
@@ -9,6 +9,17 @@ function getElementsByIds(ids) {
function testSelectorIdsMatch(selector, ids, testName) {
test(function(){
var elements = document.querySelectorAll(selector);
//assert_unreached('one: ' + JSON.stringify([...elements], null, 2) + '\n\ntwo: ' + JSON.stringify(getElementsByIds(ids), null, 2));
Copy link
Member

Choose a reason for hiding this comment

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

These logs and comments should not be included, as they will fail lint.

Copy link
Contributor

Choose a reason for hiding this comment

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

my bad, they're gone

html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html
test_follow_link(str);
});

async_test(function() {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not seeing this test fail in Firefox when I try it locally; are you?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, https://wptpr.live/27178/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html shows it still passing.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was failing in firefox for me, but it was passing in safari which it shouldnt be, so I just changed it a bit - how about now?

Copy link
Member

Choose a reason for hiding this comment

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

Looks good; failing in Firefox now! https://wptpr.live/27178/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html

@stephenmcgruer stephenmcgruer added the safe for preview This issue is safe (contains no suspect code) to be mirrored to wptpr.live label Jan 14, 2021
@github-actions github-actions bot requested a deployment to wpt-preview-27178 January 14, 2021 16:45 Pending
@stephenmcgruer stephenmcgruer added safe for preview This issue is safe (contains no suspect code) to be mirrored to wptpr.live and removed safe for preview This issue is safe (contains no suspect code) to be mirrored to wptpr.live labels Jan 14, 2021
@github-actions github-actions bot temporarily deployed to wpt-preview-27178 January 14, 2021 18:48 Inactive
@github-actions github-actions bot temporarily deployed to wpt-preview-27178 January 15, 2021 01:58 Inactive
domenic
domenic approved these changes Jan 15, 2021
View reviewed changes
html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html
test_follow_link(str);
});

async_test(function() {
Copy link
Member

Choose a reason for hiding this comment

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

Looks good; failing in Firefox now! https://wptpr.live/27178/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html

@josepharhar
Copy link
Contributor

ugh it looks like its flaky on firefox, which kinda makes sense since its a bit racy - I can't think of any better ideas of how to wait for the navigation to occur or not and pass when it doesn't occur...
I'll try to mess with it to find something that works.

@josepharhar
Copy link
Contributor

ugh it looks like its flaky on firefox, which kinda makes sense since its a bit racy - I can't think of any better ideas of how to wait for the navigation to occur or not and pass when it doesn't occur...
I'll try to mess with it to find something that works.

I think I figured it out, new patch on the way.

@github-actions github-actions bot temporarily deployed to wpt-preview-27178 January 15, 2021 19:21 Inactive
@github-actions github-actions bot temporarily deployed to wpt-preview-27178 January 15, 2021 19:59 Inactive
@josepharhar @chromium-wpt-export-bot
Update <link> pseudo selector WPTs
d0e9420 
In these spec PRs [1][2], the <link> element will no longer match :link,
:visited, or :any-link, and it will no longer navigate the page when
clicked.

This patch also modifies the TestExpectations for one of the modified
tests navigation.sub.html. There was one test case which was
consistently timing out which I removed, which would stop the test from
timing out. However, there are an additional two test cases which time
out when run with run_web_tests.py but pass just fine with
run_wpt_tests.py. In addition, the flags passed to run_web_tests.py on
the trybots make it so that those two test cases which time out get
written to a failure file instead of making the whole test runner time
out, which would count as a failure in TestExpectations. Since the
default flags for run_web_tests.py make it an actual timeout according to
TestExpectations, I left the timeout expectation in there as well.

Hopefully this test can just be run with run_wpt_tests.py in the future
and we can get rid of this TestExpectation.

[1] whatwg/html#6269
[2] w3c/csswg-drafts#5839

Bug: 611093
Change-Id: I7234eb6024450e28c1ca6ec1ede7fdfc8f2ece52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628037
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844921}
@github-actions github-actions bot temporarily deployed to wpt-preview-27178 January 19, 2021 22:17 Inactive
@github-actions github-actions bot temporarily deployed to wpt-preview-27178 January 19, 2021 22:17 Inactive
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 7fb7957 into master Jan 19, 2021
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-2628037 branch January 19, 2021 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josepharhar josepharhar josepharhar left review comments

@domenic domenic domenic approved these changes

@wpt-pr-bot wpt-pr-bot wpt-pr-bot approved these changes

Assignees
No one assigned
Labels
chromium-export dom html safe for preview This issue is safe (contains no suspect code) to be mirrored to wptpr.live
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@chromium-wpt-export-bot @josepharhar @domenic @wpt-pr-bot @stephenmcgruer