Bug 182542

Summary: Serialize font-variation-settings with double-quotes per spec
Product: WebKit Reporter: Chris Nardi <cnardi>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, koivisto, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=185166
Attachments:
Description Flags
Patch
none
Update test none

Chris Nardi
Reported 2018-02-06 10:50:20 PST
Serialize font-variation-settings with double-quotes per spec
Attachments
Patch (1.82 KB, patch)
2018-02-06 10:52 PST, Chris Nardi 		no flags
DetailsFormatted DiffDiff
Update test (6.99 KB, patch)
2018-02-06 11:00 PST, Chris Nardi 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Nardi
Comment 1 2018-02-06 10:52:53 PST
Created attachment 333190 [details] Patch
EWS Watchlist
Comment 2 2018-02-06 10:54:02 PST
Attachment 333190 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:14: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Chris Nardi
Comment 3 2018-02-06 11:00:45 PST
Created attachment 333191 [details] Update test
Chris Nardi
Comment 4 2018-04-19 10:15:03 PDT
mmaxfield@, could you review?
Myles C. Maxfield
Comment 5 2018-04-20 04:43:07 PDT
IIRC there was a previously contributed patch to change them to single quotes. We should talk with the person who wrote that patch to figure out the correct syntax.
Chris Nardi
Comment 6 2018-04-20 06:50:03 PDT
(In reply to Myles C. Maxfield from comment #5) > IIRC there was a previously contributed patch to change them to single > quotes. We should talk with the person who wrote that patch to figure out > the correct syntax. Perhaps you were thinking of bug 182201 where I changed to double quotes for font-feature-settings? (Yes, this could've been one patch, but I found this bug after the last patch landed.) Per https://drafts.csswg.org/cssom/#serialize-a-string, double quotes is correct, and I changed this behavior in Chrome a few months ago (I assume this is what Firefox does as well but their implementation of font-variation-settings is behind a flag).
Chris Nardi
Comment 7 2018-04-30 11:53:06 PDT
(In reply to Chris Nardi from comment #6) > Per https://drafts.csswg.org/cssom/#serialize-a-string, double quotes is > correct, and I changed this behavior in Chrome a few months ago (I assume > this is what Firefox does as well but their implementation of > font-variation-settings is behind a flag). Yep, Firefox does use double quotes per the spec.
WebKit Commit Bot
Comment 8 2018-04-30 12:56:33 PDT
Comment on attachment 333191 [details] Update test Clearing flags on attachment: 333191 Committed r231165: <https://trac.webkit.org/changeset/231165>
WebKit Commit Bot
Comment 9 2018-04-30 12:56:35 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2018-04-30 12:57:26 PDT
<rdar://problem/39845194>
Note You need to log in before you can comment on or make changes to this bug.