Bug 30062

Summary: Inspector should syntax highlight JS/CSS in elements view
Product: WebKit Reporter: Brian Weinstein <bweinstein>
Component: Web Inspector (Deprecated)Assignee: Brian Weinstein <bweinstein>
Status: RESOLVED FIXED    
Severity: Normal CC: joepeck, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Screen Shot
none
Fix Take 2
timothy: review-, bweinstein: commit-queue-
Screen Shot + Whitespace
none
Fix actually take 2
bweinstein: commit-queue-
No quotes or leading/trailing whitespace
timothy: review-, bweinstein: commit-queue-
Leading newlines + Trailing whitespace timothy: review+, bweinstein: commit-queue-

Description Brian Weinstein 2009-10-04 17:35:07 PDT
Created attachment 40599 [details]
Screen Shot

Inspector should syntax highlight JS/CSS in elements view.
Comment 1 Brian Weinstein 2009-10-04 17:38:46 PDT
Created attachment 40600 [details]
Fix Take 2
Comment 2 Timothy Hatcher 2009-10-04 17:39:00 PDT
Nice! We should preserve line breaks and whitespcae too.
Comment 3 Brian Weinstein 2009-10-04 17:46:37 PDT
(In reply to comment #2)
> Nice! We should preserve line breaks and whitespcae too.

Do I have a step in the code get rid of that? I'm not sure where I would be destroying whitespace. Do I manually have to add code to get it to preserve whitespace (via string replacements)?
Comment 4 Timothy Hatcher 2009-10-04 17:49:03 PDT
Comment on attachment 40600 [details]
Fix Take 2

I worry adding inspector.css to the iframe will cause subtle issues, since we have many generic selectors in that file.

I think duplicating the style rules is fine. Or move all the shareable rules from styleText into a new CSS file and make inspector.html and the iframe include that.

I am also surprised the syntax highligher objects work when passed null.
Comment 5 Timothy Hatcher 2009-10-04 17:53:45 PDT
Comment on attachment 40600 [details]
Fix Take 2

> +                    info.title ="\"<span>" + newNode.innerHTML + "</span>\"";
> +                    info.title ="\"<span>" + newNode.innerHTML + "</span>\"";

You should put class=\"webkit-html-text-node\" on these spans too. Missing a space after the "=".

> +                } else if (this.parentNode && this.parentNode.nodeName.toLowerCase().escapeHTML() == "style") {

No need for .escapeHTML(), since it will never have HTML characters.

Try adding "white-space: pre" to the webkit-html-text-node class, and see if that preserves line breaks. It might break things, but worth a try.
Comment 6 Brian Weinstein 2009-10-04 20:44:22 PDT
Created attachment 40603 [details]
Screen Shot + Whitespace
Comment 7 Brian Weinstein 2009-10-04 20:45:04 PDT
Created attachment 40604 [details]
Fix actually take 2
Comment 8 Timothy Hatcher 2009-10-04 20:50:12 PDT
Comment on attachment 40604 [details]
Fix actually take 2

This looks good! But I think we want to trim starting newlines and ending whitespace to make it look less odd.

Also remove the quotes?
Comment 9 Joseph Pecoraro 2009-10-04 20:55:13 PDT
Yah, this looks great (I applied it locally and it works great)!

> Add syntax higlighting of CSS and Javascript tags to the elements panel.

A few typos in the ChangeLog that you might want to fix. "highlighting" and "JavaScript".

(In reply to comment #8)
> (From update of attachment 40604 [details])
> This looks good! But I think we want to trim starting newlines and ending
> whitespace to make it look less odd.
> 
> Also remove the quotes?

+1
Comment 10 Brian Weinstein 2009-10-04 21:00:18 PDT
Created attachment 40605 [details]
No quotes or leading/trailing whitespace
Comment 11 Timothy Hatcher 2009-10-04 21:05:24 PDT
Comment on attachment 40605 [details]
No quotes or leading/trailing whitespace

You only want to remove newlines from the beginning, so that indented text matches up with other lines.

So:

/^[\n\r]*/
Comment 12 Brian Weinstein 2009-10-04 21:11:01 PDT
Created attachment 40607 [details]
Leading newlines + Trailing whitespace
Comment 13 Brian Weinstein 2009-10-04 21:17:57 PDT
Committed in http://trac.webkit.org/changeset/49081.