Bug 206831

Summary: [css-grid] Move grid-item-alignment tests to WPT folder
Product: WebKit Reporter: rmonteriso
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, commit-queue, ews-watchlist, jfernandez, rego, svillar, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

rmonteriso
Reported 2020-01-27 10:03:48 PST
This patch moves the following tests: * grid-item-alignment-with-orthogonal-flows.html * grid-item-alignment-with-orthogonal-flows-vertical-rl.html * grid-item-alignment-with-orthogonal-flows-vertical-lr.html and removes their corresponding outdated versions from css-grid-layout folder. Furthermore, .thirdRowFirstColumn class is added to grid.css support file and removed from the single <style> section of the tests using this class.
Attachments
Patch (52.49 KB, patch)
2020-01-27 10:17 PST, rmonteriso 		no flags
DetailsFormatted DiffDiff
Patch (56.43 KB, patch)
2020-01-29 03:15 PST, rmonteriso 		no flags
DetailsFormatted DiffDiff
Patch (56.33 KB, patch)
2020-01-29 04:37 PST, rmonteriso 		no flags
DetailsFormatted DiffDiff
Patch (56.27 KB, patch)
2020-01-29 06:17 PST, rmonteriso 		no flags
DetailsFormatted DiffDiff
Patch (56.25 KB, patch)
2020-01-29 09:03 PST, rmonteriso 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
rmonteriso
Comment 1 2020-01-27 10:17:35 PST
Created attachment 388870 [details] Patch
Carlos Alberto Lopez Perez
Comment 2 2020-01-28 07:49:48 PST
Comment on attachment 388870 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388870&action=review > LayoutTests/imported/w3c/ChangeLog:10 > + [css-grid] Move grid-item-alignment tests to WPT folder > + https://bugs.webkit.org/show_bug.cgi?id=206831 > + > + Reviewed by NOBODY (OOPS!). > + > + Add grid-item-alignment tests, checked and adapted, to WPT. > + Add .thirdRowFirstColumn class to grid.css support file and update all tests using this class by removing the duplicated class > + from their <style> section. I guess this changes have been already exported to WPT, and this is a manual import of this tests, right? In that case, telling in the changelog from which WPT commit this changes come can be useful.
rmonteriso
Comment 3 2020-01-29 03:06:18 PST
(In reply to Carlos Alberto Lopez Perez from comment #2) > Comment on attachment 388870 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=388870&action=review > > > LayoutTests/imported/w3c/ChangeLog:10 > > + [css-grid] Move grid-item-alignment tests to WPT folder > > + https://bugs.webkit.org/show_bug.cgi?id=206831 > > + > > + Reviewed by NOBODY (OOPS!). > > + > > + Add grid-item-alignment tests, checked and adapted, to WPT. > > + Add .thirdRowFirstColumn class to grid.css support file and update all tests using this class by removing the duplicated class > > + from their <style> section. > > I guess this changes have been already exported to WPT, and this is a manual > import of this tests, right? > In that case, telling in the changelog from which WPT commit this changes > come can be useful. Thanks, I will add the Chromium CL with which the WPT repo has been updated.
rmonteriso
Comment 4 2020-01-29 03:15:22 PST
Created attachment 389118 [details] Patch
rmonteriso
Comment 5 2020-01-29 04:37:16 PST
Created attachment 389120 [details] Patch
rmonteriso
Comment 6 2020-01-29 06:17:35 PST
Created attachment 389131 [details] Patch
Carlos Alberto Lopez Perez
Comment 7 2020-01-29 06:21:24 PST
Comment on attachment 389131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389131&action=review > LayoutTests/imported/w3c/ChangeLog:11 > + Imported to WPT with Chromium CL https://chromium-review.googlesource.com/c/chromium/src/+/2022793 nitpick: this should mention the WPT commit (not the Chromium CL). I guess its https://github.com/web-platform-tests/wpt/commit/b628488204 ?
rmonteriso
Comment 8 2020-01-29 08:41:39 PST
(In reply to Carlos Alberto Lopez Perez from comment #7) > Comment on attachment 389131 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=389131&action=review > > > LayoutTests/imported/w3c/ChangeLog:11 > > + Imported to WPT with Chromium CL https://chromium-review.googlesource.com/c/chromium/src/+/2022793 > > nitpick: this should mention the WPT commit (not the Chromium CL). I guess > its https://github.com/web-platform-tests/wpt/commit/b628488204 ? The PR you mention here is related to other css tests, already reimported to WebKit. The tests contained in this patch, instead, have been directly imported with the Chromium CL, without a previous PR to WPT. Is there a better way I could rephrase this?
Carlos Alberto Lopez Perez
Comment 9 2020-01-29 08:47:22 PST
Comment on attachment 389131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389131&action=review >>> LayoutTests/imported/w3c/ChangeLog:11 >>> + Imported to WPT with Chromium CL https://chromium-review.googlesource.com/c/chromium/src/+/2022793 >> >> nitpick: this should mention the WPT commit (not the Chromium CL). I guess its https://github.com/web-platform-tests/wpt/commit/b628488204 ? > > The PR you mention here is related to other css tests, already reimported to WebKit. > The tests contained in this patch, instead, have been directly imported with the Chromium CL, without a previous PR to WPT. Is there a better way I could rephrase this? Yeah, it seems I got the wrong commit :) I guess including the link to the WPT PR from to chromium CL would be nice. Is this one https://github.com/web-platform-tests/wpt/pull/21440 ?
rmonteriso
Comment 10 2020-01-29 08:52:01 PST
(In reply to Carlos Alberto Lopez Perez from comment #9) > Comment on attachment 389131 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=389131&action=review > > >>> LayoutTests/imported/w3c/ChangeLog:11 > >>> + Imported to WPT with Chromium CL https://chromium-review.googlesource.com/c/chromium/src/+/2022793 > >> > >> nitpick: this should mention the WPT commit (not the Chromium CL). I guess its https://github.com/web-platform-tests/wpt/commit/b628488204 ? > > > > The PR you mention here is related to other css tests, already reimported to WebKit. > > The tests contained in this patch, instead, have been directly imported with the Chromium CL, without a previous PR to WPT. Is there a better way I could rephrase this? > > Yeah, it seems I got the wrong commit :) > I guess including the link to the WPT PR from to chromium CL would be nice. > Is this one https://github.com/web-platform-tests/wpt/pull/21440 ? A-ha, I see! Yes it is, I'm going to add it with the next patch!
rmonteriso
Comment 11 2020-01-29 09:03:33 PST
Created attachment 389144 [details] Patch
WebKit Commit Bot
Comment 12 2020-01-30 02:27:29 PST
Comment on attachment 389144 [details] Patch Clearing flags on attachment: 389144 Committed r255419: <https://trac.webkit.org/changeset/255419>
WebKit Commit Bot
Comment 13 2020-01-30 02:27:31 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 14 2020-01-30 02:28:16 PST
<rdar://problem/59023261>
Note You need to log in before you can comment on or make changes to this bug.