Bug 182542 - Serialize font-variation-settings with double-quotes per spec
Summary: Serialize font-variation-settings with double-quotes per spec
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-06 10:50 PST by Chris Nardi
Modified: 2018-05-01 12:46 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.82 KB, patch)
2018-02-06 10:52 PST, Chris Nardi
no flags Details | Formatted Diff | Diff
Update test (6.99 KB, patch)
2018-02-06 11:00 PST, Chris Nardi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Nardi 2018-02-06 10:50:20 PST
Serialize font-variation-settings with double-quotes per spec
Comment 1 Chris Nardi 2018-02-06 10:52:53 PST
Created attachment 333190 [details]
Patch
Comment 2 Build Bot 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.
Comment 3 Chris Nardi 2018-02-06 11:00:45 PST
Created attachment 333191 [details]
Update test
Comment 4 Chris Nardi 2018-04-19 10:15:03 PDT
mmaxfield@, could you review?
Comment 5 Myles C. Maxfield 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.
Comment 6 Chris Nardi 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).
Comment 7 Chris Nardi 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.
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2018-04-30 12:56:35 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-04-30 12:57:26 PDT
<rdar://problem/39845194>