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 treegraph
 
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, Build Bot
no flags Details

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 Build Bot 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 Build Bot 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