Bug 204508

Summary:

[css-grid] Serialization of grid-area, grid-row and grid-column should include "/" separator

Product:

WebKit

Reporter:

Manuel Rego Casasnovas <rego>

Component:

CSS

Assignee:

Manuel Rego Casasnovas <rego>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

clinton.ayres, commit-queue, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jfernandez, macpherson, menard, obrufau, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

204611

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews214 for win-future	none

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