224601 – [selectors] Update :focus-visible tests from WPT

RESOLVED FIXED 224601

[selectors] Update :focus-visible tests from WPT

https://bugs.webkit.org/show_bug.cgi?id=224601

Summary [selectors] Update :focus-visible tests from WPT

Manuel Rego Casasnovas

Reported 2021-04-15 05:28:25 PDT

[selectors] Update :focus-visible tests from WPT

Attachments
Patch (102.11 KB, patch) 2021-04-15 05:28 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch (104.51 KB, patch) 2021-04-15 06:10 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch (104.86 KB, patch) 2021-04-15 07:23 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch (107.94 KB, patch) 2021-04-15 11:57 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch (108.16 KB, patch) 2021-04-15 22:08 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch (108.22 KB, patch) 2021-04-15 22:34 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch for landing (106.42 KB, patch) 2021-04-16 01:07 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch (4.80 KB, patch) 2021-04-18 22:09 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Patch (12.11 KB, patch) 2021-04-18 22:16 PDT, Manuel Rego Casasnovas	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Manuel Rego Casasnovas

Comment 1 2021-04-15 05:28:56 PDT

Created attachment 426097 [details] Patch

Manuel Rego Casasnovas

Comment 2 2021-04-15 06:10:00 PDT

Created attachment 426101 [details] Patch

Manuel Rego Casasnovas

Comment 3 2021-04-15 07:23:51 PDT

Created attachment 426103 [details] Patch

Darin Adler

Comment 4 2021-04-15 10:27:45 PDT

Comment on attachment 426103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426103&action=review > LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-002.html:33 > - This test checks that <code>:focus-visible</code> always matches on <code><input></code> elements which take text input, regardless of focus mechanism. > + This test checks that <code>:focus-visible</code> always matches on <code><input></code> elements which take text input, regardless of focus mechanism. Do we know who made this type of change in WPT and why?

Manuel Rego Casasnovas

Comment 5 2021-04-15 11:57:00 PDT

Created attachment 426122 [details] Patch

Manuel Rego Casasnovas

Comment 6 2021-04-15 11:57:37 PDT

(In reply to Darin Adler from comment #4) > Comment on attachment 426103 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426103&action=review > > > LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-002.html:33 > > - This test checks that <code>:focus-visible</code> always matches on <code><input></code> elements which take text input, regardless of focus mechanism. > > + This test checks that <code>:focus-visible</code> always matches on <code><input></code> elements which take text input, regardless of focus mechanism. > > Do we know who made this type of change in WPT and why? The test in WPT hasn't changed, so maybe it's the import script doing the change now.

Darin Adler

Comment 7 2021-04-15 12:15:38 PDT

(In reply to Manuel Rego Casasnovas from comment #6) > The test in WPT hasn't changed, so maybe it's the import script doing the > change now. Can you research this? Like changes have there been to the import script since we imported this correctly? I’m really concerned about damaging the tests like this while importing them. For now it might be harmless? I would like to help fix this.

Manuel Rego Casasnovas

Comment 8 2021-04-15 12:40:28 PDT

(In reply to Darin Adler from comment #7) > (In reply to Manuel Rego Casasnovas from comment #6) > > The test in WPT hasn't changed, so maybe it's the import script doing the > > change now. > > Can you research this? Like changes have there been to the import script > since we imported this correctly? I’m really concerned about damaging the > tests like this while importing them. For now it might be harmless? > > I would like to help fix this. Yeah, I'll investigate it, and report my findings on a separated bug. Maybe the first time I copied the tests manually instead of using the import (cannot remember, though I usually go with the import script), or maybe there were some recent change on the import script.

Manuel Rego Casasnovas

Comment 9 2021-04-15 22:08:19 PDT

Created attachment 426182 [details] Patch

Manuel Rego Casasnovas

Comment 10 2021-04-15 22:34:04 PDT

Created attachment 426184 [details] Patch

Manuel Rego Casasnovas

Comment 11 2021-04-16 01:07:04 PDT

Created attachment 426193 [details] Patch for landing

Manuel Rego Casasnovas

Comment 12 2021-04-16 01:09:38 PDT

(In reply to Manuel Rego Casasnovas from comment #8) > (In reply to Darin Adler from comment #7) > > (In reply to Manuel Rego Casasnovas from comment #6) > > > The test in WPT hasn't changed, so maybe it's the import script doing the > > > change now. > > > > Can you research this? Like changes have there been to the import script > > since we imported this correctly? I’m really concerned about damaging the > > tests like this while importing them. For now it might be harmless? > > > > I would like to help fix this. > > Yeah, I'll investigate it, and report my findings on a separated bug. > > Maybe the first time I copied the tests manually instead of using the import > (cannot remember, though I usually go with the import script), or maybe > there were some recent change on the import script. This was an issue in Python 3.5 or bigger, reported it with a fix: https://bugs.webkit.org/show_bug.cgi?id=224658 I've used python2 on the last patch, so we avoid those modifications. Nice catch Darin.

EWS

Comment 13 2021-04-16 01:53:42 PDT

Committed r276127 (236622@main): <https://commits.webkit.org/236622@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426193 [details].

Darin Adler

Comment 14 2021-04-16 10:45:16 PDT

Looks to me like the thing we landed does still have the changes due to the bug in the expected files!

Darin Adler

Comment 15 2021-04-16 10:46:02 PDT

I’m concerned that some of the files now have incorrect expectations in them, but are also skipped. Not a great state to be in.

Darin Adler

Comment 16 2021-04-16 10:46:43 PDT

One example: web-platform-tests/css/selectors/focus-visible-002-expected.txt I don’t think this should have been changed at all as part of this import.

Manuel Rego Casasnovas

Comment 17 2021-04-18 22:09:46 PDT

Reopening to attach new patch.

Manuel Rego Casasnovas

Comment 18 2021-04-18 22:09:49 PDT

Created attachment 426394 [details] Patch

Radar WebKit Bug Importer

Comment 19 2021-04-18 22:10:08 PDT

<rdar://problem/76827947>

Manuel Rego Casasnovas

Comment 20 2021-04-18 22:16:11 PDT

Created attachment 426395 [details] Patch

EWS

Comment 21 2021-04-18 23:57:22 PDT

Committed r276237 (236719@main): <https://commits.webkit.org/236719@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426395 [details].

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component New Bugs

Assignee

Manuel Rego Casasnovas

Reported

2021-04-15 05:28 PDT

Modified

2021-04-18 23:57 PDT History

CC List

8 users Show

URL

Keywords InRadar

Depends on

Blocks

224598

Dependencies

tree graph