Bug 30062 - Inspector should syntax highlight JS/CSS in elements view
Summary: Inspector should syntax highlight JS/CSS in elements view
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Brian Weinstein
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-04 17:35 PDT by Brian Weinstein
Modified: 2009-10-04 21:17 PDT (History)
2 users (show)

See Also:

Attachments
Screen Shot (296.78 KB, image/png)
2009-10-04 17:35 PDT, Brian Weinstein 		no flags Details
Fix Take 2 (7.05 KB, patch)
2009-10-04 17:38 PDT, Brian Weinstein 		timothy: review-
bweinstein: commit-queue-
Details | Formatted Diff | Diff
Screen Shot + Whitespace (260.36 KB, image/png)
2009-10-04 20:44 PDT, Brian Weinstein 		no flags Details
Fix actually take 2 (4.10 KB, patch)
2009-10-04 20:45 PDT, Brian Weinstein 		bweinstein: commit-queue-
Details | Formatted Diff | Diff
No quotes or leading/trailing whitespace (4.17 KB, patch)
2009-10-04 21:00 PDT, Brian Weinstein 		timothy: review-
bweinstein: commit-queue-
Details | Formatted Diff | Diff
Leading newlines + Trailing whitespace (4.17 KB, patch)
2009-10-04 21:11 PDT, Brian Weinstein 		timothy: review+
bweinstein: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (4) 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.
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.