Bug 30907 – Web Inspector: Rewrite CSSSourceSyntaxHighlighter so it shares more code

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Keishi Hattori]

We should rewrite CSSSourceSyntaxHighlighter to share code with JavaScriptSourceSyntaxHighlighter.

[Keishi Hattori]

Created attachment 42126 [details]
proposed patch

Code became uglier than I hoped. This version features full list of keywords so users can spot their typos.

[Keishi Hattori]

Created attachment 42290 [details]
proposed patch 2

I think I was able to make it much more organized. Patch also fixes few issues with JS syntax highlighter.

[Timothy Hatcher]

Comment on attachment 42290 [details]
proposed patch 2

> +    const urlPattern = /^url\(\s*(?:(?:"(?:[\t !#\$%&\(-~]|\\n|\r\n|\r|\f|'|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*"|'(?:[\t !#\$%&\(-~]|\\n|\r\n|\r|\f|'|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*')|(?:[!#$%&*-~]|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*)\s*\)/i;
> +    const stringPattern = /^(?:"(?:[\t !#\$%&\(-~]|\\n|\r\n|\r|\f|'|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*"|'(?:[\t !#\$%&\(-~]|\\n|\r\n|\r|\f|'|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*')/i;
> +    const identPattern = /^-?(?:[_a-zA-Z]|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))(?:[_a-zA-Z0-9-]|[\200-\377]|(?:\[0-9a-fA-F]{1,6}[ \n\r\t\f]?|\[ -~\200-\377\n]))*/i;

The only thing I think needs fixed about this patch is the regexs. They are super complex.

There are a number of places than can be simplified. Like: [ \n\r\t\f] should just be \s.

There is also a mistake with \\n being used when it should be \n.

Also [\200-\377] is odd. I know it is fro mthe spec, but the spec also says:

> The "\377" represents the highest character number that current versions of
> Flex can deal with (decimal 255). It should be read as "\4177777" (decimal
> 1114111), which is the highest possible code point in Unicode/ISO-10646.

We can support full unicode.

The multiple character rnages ORed together should just be combined inot one character range. Like the follow example.

> [_a-zA-Z0-9-]|[\200-\377]

So you would have:

[-_a-zA-Z0-9\200-\4177777]

I wonder if we should just use \w and call it a day?


This \ should be \\.

\[0-9a-fA-F]{1,6}

It is really hard to review this. These are just the things that stood out to me. I recomend we just write simple regexs and not try to generate them from the spec's FLEX tokenizer.

[Keishi Hattori]

Created attachment 42293 [details]
proposed patch 3

Simplified the regexps.

[Timothy Hatcher]

Comment on attachment 42293 [details]
proposed patch 3

propertyKeywords, valueKeywords, mediaTypes and keywords should be objects with properties not arrays. That way it is fast to look up.

So:

const mediaTypes = {"all": true, "aural": true, "braille": true, "embossed": true, "handheld": true, "print": true, "projection": true, "screen": true, "tty": true, "tv": true};

And test like:

token in mediaTypes

[Keishi Hattori]

Created attachment 42298 [details]
proposed patch 4

[Timothy Hatcher]

I don't see a difference between 3 and 4.

[Keishi Hattori]

Created attachment 42303 [details]
Sorry, I was editing the wrong file

[WebKit Commit Bot]

Comment on attachment 42303 [details]
Sorry, I was editing the wrong file

Clearing flags on attachment: 42303

Committed r50411: <http://trac.webkit.org/changeset/50411>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.