Bug 206831 – [css-grid] Move grid-item-alignment tests to WPT folder

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

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.

Created attachment 388870 [details]
Patch

[Carlos Alberto Lopez Perez]

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.

(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.

Created attachment 389118 [details]
Patch

Created attachment 389120 [details]
Patch

Created attachment 389131 [details]
Patch

[Carlos Alberto Lopez Perez]

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 ?

(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 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 ?

(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!

Created attachment 389144 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 389144 [details]
Patch

Clearing flags on attachment: 389144

Committed r255419: <https://trac.webkit.org/changeset/255419>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59023261>