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-

Brian Weinstein
Reported 2009-10-04 17:35:07 PDT
Created attachment 40599 [details] Screen Shot Inspector should syntax highlight JS/CSS in elements view.
Attachments
Screen Shot (296.78 KB, image/png)
2009-10-04 17:35 PDT, Brian Weinstein
no flags
Fix Take 2 (7.05 KB, patch)
2009-10-04 17:38 PDT, Brian Weinstein
timothy: review-
bweinstein: commit-queue-
Screen Shot + Whitespace (260.36 KB, image/png)
2009-10-04 20:44 PDT, Brian Weinstein
no flags
Fix actually take 2 (4.10 KB, patch)
2009-10-04 20:45 PDT, Brian Weinstein
bweinstein: commit-queue-
No quotes or leading/trailing whitespace (4.17 KB, patch)
2009-10-04 21:00 PDT, Brian Weinstein
timothy: review-
bweinstein: commit-queue-
Leading newlines + Trailing whitespace (4.17 KB, patch)
2009-10-04 21:11 PDT, Brian Weinstein
timothy: review+
bweinstein: commit-queue-
Brian Weinstein
Comment 1 2009-10-04 17:38:46 PDT
Created attachment 40600 [details] Fix Take 2
Timothy Hatcher
Comment 2 2009-10-04 17:39:00 PDT
Nice! We should preserve line breaks and whitespcae too.
Brian Weinstein
Comment 3 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)?
Timothy Hatcher
Comment 4 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.
Timothy Hatcher
Comment 5 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.
Brian Weinstein
Comment 6 2009-10-04 20:44:22 PDT
Created attachment 40603 [details] Screen Shot + Whitespace
Brian Weinstein
Comment 7 2009-10-04 20:45:04 PDT
Created attachment 40604 [details] Fix actually take 2
Timothy Hatcher
Comment 8 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?
Joseph Pecoraro
Comment 9 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
Brian Weinstein
Comment 10 2009-10-04 21:00:18 PDT
Created attachment 40605 [details] No quotes or leading/trailing whitespace
Timothy Hatcher
Comment 11 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]*/
Brian Weinstein
Comment 12 2009-10-04 21:11:01 PDT
Created attachment 40607 [details] Leading newlines + Trailing whitespace
Brian Weinstein
Comment 13 2009-10-04 21:17:57 PDT
Note You need to log in before you can comment on or make changes to this bug.