Bug 30831 – Web Inspector: syntax highlight inline JS and CSS in HTML resources

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

[Timothy Hatcher]

We should syntax highlight <script> and <style> blocks in HTML resources.

[Anthony Ricaud]

*** Bug 19351 has been marked as a duplicate of this bug. ***

[Brian Weinstein]

Can we do this in the inspector, or would it have to be done in the engine?

[Timothy Hatcher]

We could do this in the Inspector.

[Keishi Hattori]

I'll take this after I'm done with the new CSS syntax highlighter.

[Alexander Pavlov (apavlov)]

SourceHTMLTokenizer implements tokenization of the <script> tag contents with a SourceJavaScriptTokenizer, so we are good with that. We should implement a similar approach for the <style> tag and SourceCSSTokenizer.

[Alexander Pavlov (apavlov)]

Created attachment 80204 [details]
[PATCH] Suggested solution

[Yury Semikhatsky]

Comment on attachment 80204 [details]
[PATCH] Suggested solution

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

> Source/WebCore/inspector/front-end/SourceHTMLTokenizer.re2js:267
>                      if (this._condition.parseCondition & this._parseConditions.SCRIPT) {

You should add a check for STYLE state here, r- for this.

[Alexander Pavlov (apavlov)]

Created attachment 80436 [details]
[PATCH] Comment addressed, added STYLE check in 2 places

[Yury Semikhatsky]

Comment on attachment 80436 [details]
[PATCH] Comment addressed, added STYLE check in 2 places

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

> Source/WebCore/inspector/front-end/SourceHTMLTokenizer.re2js:268
>                          // Do not tokenize script tag contents, keep lexer state although processing "<".

Please update the comment to reflect that you're skipping style tag content as well.

> Source/WebCore/inspector/front-end/SourceHTMLTokenizer.re2js:322
> +                    if (this._condition.parseCondition === this._parseConditions.SCRIPT || this._condition.parseCondition === this._parseConditions.STYLE) {

We should be consistent in using === vs & if there is no difference in case of SCRIPT and STYLE parse conditions I'd prefer ===

[Alexander Pavlov (apavlov)]

(In reply to comment #9)
> (From update of attachment 80436 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80436&action=review
> 
> > Source/WebCore/inspector/front-end/SourceHTMLTokenizer.re2js:268
> >                          // Do not tokenize script tag contents, keep lexer state although processing "<".
> 
> Please update the comment to reflect that you're skipping style tag content as well.

Done

> > Source/WebCore/inspector/front-end/SourceHTMLTokenizer.re2js:322
> > +                    if (this._condition.parseCondition === this._parseConditions.SCRIPT || this._condition.parseCondition === this._parseConditions.STYLE) {
> 
> We should be consistent in using === vs & if there is no difference in case of SCRIPT and STYLE parse conditions I'd prefer ===

Alas, there is a difference. At times, we can have the ATTRIBUTE_VALUE or ATTRIBUTE flag set along with the SCRIPT or STYLE state (I have seen this break the highlighting). There is a special comment following the line quoted: "Fall through if expecting attributes", which mean this should not work if the ATTRIBUTE flag is set, so there is no point in fixing the consistency.

[Alexander Pavlov (apavlov)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       Source/WebCore/ChangeLog
        M       Source/WebCore/inspector/front-end/SourceHTMLTokenizer.js
        M       Source/WebCore/inspector/front-end/SourceHTMLTokenizer.re2js
Committed r76943

[Alexander Pavlov (apavlov)]

Comment on attachment 80436 [details]
[PATCH] Comment addressed, added STYLE check in 2 places

Clearing cq? from a committed patch