Bug 126337 - Implement `CSS.escape` as per CSSOM
Summary: Implement `CSS.escape` as per CSSOM
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL: http://dev.w3.org/csswg/cssom/#the-cs...
Keywords:
Depends on:
Blocks: 151378
  Show dependency tree / graph
 
Reported: 2013-12-31 06:50 PST by Mathias Bynens
Modified: 2016-08-27 17:23 PDT (History)
12 users (show)

See Also:

Attachments
[PATCH] Proposed Fix (17.08 KB, patch)
2016-08-23 13:59 PDT, Joseph Pecoraro 		no flags Details | Formatted Diff | Diff
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.
Comment 1 Mathias Bynens 2015-09-15 07:31:58 PDT 
FWIW, this recently landed in Chromium.
Comment 2 Nikita Vasilyev 2015-11-17 19:22:52 PST 
*** Bug 149175 has been marked as a duplicate of this bug. ***
Comment 3 Joseph Pecoraro 2016-08-23 11:57:06 PDT 
We are considering using the polyfill in Web Inspector at the moment on bug 151378.
Comment 4 Joseph Pecoraro 2016-08-23 12:02:25 PDT 
The Blink commit that included this looks quite small. It would probably be trivial to port it over to WebKit:
<https://chromium.googlesource.com/chromium/blink/+/c1a5ffdc924b089e70cd33ad2726b58cc8312abe%5E!/>
Comment 5 Mathias Bynens 2016-08-23 13:34:21 PDT 
(In reply to comment #4)
> The Blink commit that included this looks quite small. It would probably be
> trivial to port it over to WebKit:
> <https://chromium.googlesource.com/chromium/blink/+/
> c1a5ffdc924b089e70cd33ad2726b58cc8312abe%5E!/>

Keep in mind that various follow-up fixes were committed separately, so be sure to base any ports on the latest code in Blink rather than that individual commit.
Comment 6 Joseph Pecoraro 2016-08-23 13:43:43 PDT 
I'll take this.
Comment 7 Joseph Pecoraro 2016-08-23 13:59:15 PDT 
Created attachment 286768 [details]
[PATCH] Proposed Fix
Comment 8 Joseph Pecoraro 2016-08-23 14:01:27 PDT 
Comment on attachment 286768 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/ChangeLog:14
> +        (WebCore::serializeIdentifier):
> +        Update serialization to match the latest version of the spec:
> +        <https://drafts.csswg.org/cssom/#serialize-an-identifier>
> +        New handling for 0x0, 0x7f, just "-", and "--" is now allowed.

Note: WebCore::serializeString can use the same handling updates for 0x0 and 0x7f but I was unsure how to test those.
Comment 9 WebKit Commit Bot 2016-08-24 19:42:42 PDT 
Comment on attachment 286768 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 286768

Committed r204952: <http://trac.webkit.org/changeset/204952>
Comment 10 WebKit Commit Bot 2016-08-24 19:42:46 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 Darin Adler 2016-08-27 17:21:56 PDT 
Comment on attachment 286768 [details]
[PATCH] Proposed Fix

The characterStartingAt plus U16_LENGTH idiom is ugly. I suggest we use StringView::codePoints instead.
Comment 12 Darin Adler 2016-08-27 17:23:00 PDT 
In some other tests like this I changed the tests so we could see non-trivial characters in hex. This made it easier to see the tests were correct for unusual characters.