Bug 107044

Summary:

[CSS Grid Layout] Add grid.css to hold the common grid testing code

Product:

WebKit

Reporter:

Julien Chaffraix <jchaffraix>

Component:

Tools / Tests

Assignee:

Julien Chaffraix <jchaffraix>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

donggwan.kim, ojan, tony, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

60731

Attachments:

Description	Flags
Proposed change 1: Added grid.css and move most files to it.	none
Proposed change 2: Updated the class names per Tony's request.	none
Proposed change 3: s/id/class/g.	none

Julien Chaffraix

Reported 2013-01-16 12:41:10 PST

Good suggestion from Tony! This should be similar to what is done in flexbox.css. The idea is to centralize some of the definitions to avoid having to touch lots of files when unprefixing or updating the syntax (e.g. see bug 103336 and bug 103338).

Attachments
Proposed change 1: Added grid.css and move most files to it. (36.12 KB, patch) 2013-01-18 17:10 PST, Julien Chaffraix	no flags	Details Formatted Diff Diff
Proposed change 2: Updated the class names per Tony's request. (75.08 KB, patch) 2013-01-22 12:58 PST, Julien Chaffraix	no flags	Details Formatted Diff Diff
Proposed change 3: s/id/class/g. (75.30 KB, patch) 2013-01-22 14:49 PST, Julien Chaffraix	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Julien Chaffraix

Comment 1 2013-01-18 17:10:07 PST

Created attachment 183578 [details] Proposed change 1: Added grid.css and move most files to it.

Julien Chaffraix

Comment 2 2013-01-20 14:03:17 PST

*** Bug 104869 has been marked as a duplicate of this bug. ***

Tony Chang

Comment 3 2013-01-22 10:17:29 PST

Comment on attachment 183578 [details] Proposed change 1: Added grid.css and move most files to it. View in context: https://bugs.webkit.org/attachment.cgi?id=183578&action=review > LayoutTests/fast/css-grid-layout/resources/grid.css:11 > +#a { I think using single letter ids seems like it could cause problems and weird side effects in the future. Can we make these classes with descriptive names?

Julien Chaffraix

Comment 4 2013-01-22 12:58:34 PST

Created attachment 184037 [details] Proposed change 2: Updated the class names per Tony's request.

Tony Chang

Comment 5 2013-01-22 13:48:34 PST

Julien Chaffraix

Comment 6 2013-01-22 14:42:13 PST

Comment on attachment 184037 [details] Proposed change 2: Updated the class names per Tony's request. View in context: https://bugs.webkit.org/attachment.cgi?id=184037&action=review >> LayoutTests/fast/css-grid-layout/resources/grid.css:11 >> +#firstRowFirstColumn { > > These should be class names, no? You have test cases that have multiple divs with the same id, which is confusing. Not really, the tests have had several divs with the same id for some time. From a CSS selector perspective, it is fine. Where it will get hairy is if you try to query the id from JavaScript. I agree that this should be changed and I might as well pile it on this patch.

Julien Chaffraix

Comment 7 2013-01-22 14:49:49 PST

Created attachment 184051 [details] Proposed change 3: s/id/class/g.

WebKit Review Bot

Comment 8 2013-01-22 15:34:47 PST

Comment on attachment 184051 [details] Proposed change 3: s/id/class/g. Clearing flags on attachment: 184051 Committed r140480: <http://trac.webkit.org/changeset/140480>

WebKit Review Bot

Comment 9 2013-01-22 15:34:51 PST

All reviewed patches have been landed. Closing bug.

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