Bug 204508 - [css-grid] Serialization of grid-area, grid-row and grid-column should include "/" separator
Summary: [css-grid] Serialization of grid-area, grid-row and grid-column should includ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords: InRadar
: 197067 (view as bug list)
Depends on:
Blocks: 204611
  Show dependency tree / graph
 
Reported: 2019-11-22 04:05 PST by Manuel Rego Casasnovas
Modified: 2019-11-27 07:09 PST (History)
12 users (show)

See Also:

Attachments
Patch (35.73 KB, patch)
2019-11-26 04:13 PST, Manuel Rego Casasnovas 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews214 for win-future (15.52 MB, application/zip)
2019-11-26 10:19 PST, EWS Watchlist 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel Rego Casasnovas 2019-11-22 04:05:22 PST 
Currently this test fails http://w3c-test.org/css/css-grid/parsing/grid-area-valid.html because of serialization of grid-area, grid-row and grid-column doesn't include the "/" separator.

This works fine in Chromium and Firefox.
Comment 1 Manuel Rego Casasnovas 2019-11-26 04:13:19 PST 
Created attachment 384345 [details]
Patch
Comment 2 EWS Watchlist 2019-11-26 10:19:04 PST 
Comment on attachment 384345 [details]
Patch

Attachment 384345 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13279838

New failing tests:
css3/filters/blur-various-radii.html
Comment 3 EWS Watchlist 2019-11-26 10:19:07 PST 
Created attachment 384365 [details]
Archive of layout-test-results from ews214 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews214  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 4 Javier Fernandez 2019-11-27 00:39:18 PST 
Comment on attachment 384345 [details]
Patch

r=me as long as the bots are green before landing.
Comment 5 Manuel Rego Casasnovas 2019-11-27 04:01:40 PST 
Comment on attachment 384345 [details]
Patch

Windows EWS failure is unrelated and the rest of EWS are green.
Comment 6 WebKit Commit Bot 2019-11-27 04:44:26 PST 
Comment on attachment 384345 [details]
Patch

Clearing flags on attachment: 384345

Committed r252901: <https://trac.webkit.org/changeset/252901>
Comment 7 WebKit Commit Bot 2019-11-27 04:44:28 PST 
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2019-11-27 04:45:25 PST 
<rdar://problem/57504966>
Comment 9 Oriol Brufau 2019-11-27 05:02:42 PST 
*** Bug 197067 has been marked as a duplicate of this bug. ***
Comment 10 Darin Adler 2019-11-27 06:02:11 PST 
Comment on attachment 384345 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=384345&action=review

> Source/WebCore/css/StyleProperties.h:163
> +    String getShorthandValue(const StylePropertyShorthand&, const String& separator = " ") const;

Using a string object here is unnecessarily inefficient, creating and destroying a string every time this function is called. The argument should be typed const char* instead.Nothing else about the code would need to change.
Comment 11 Manuel Rego Casasnovas 2019-11-27 07:09:24 PST 
(In reply to Darin Adler from comment #10)
> Comment on attachment 384345 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=384345&action=review
> 
> > Source/WebCore/css/StyleProperties.h:163
> > +    String getShorthandValue(const StylePropertyShorthand&, const String& separator = " ") const;
> 
> Using a string object here is unnecessarily inefficient, creating and
> destroying a string every time this function is called. The argument should
> be typed const char* instead.Nothing else about the code would need to
> change.

Sorry about that, I've a patch to fix it at https://bugs.webkit.org/show_bug.cgi?id=204644