Closed
Bug 160924
Opened 23 years ago
Closed 23 years ago
View Selection Source should check to see if syntax highlighting is enabled
Categories
(Core Graveyard :: View Source, defect)
Core Graveyard
View Source
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: stdowa+bugzilla, Assigned: neil)
References
()
Details
Attachments
(1 file, 1 obsolete file)
|
3.50 KB,
patch
|
doronr
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
The menuitem for syntax highlighting in View Selection Source should be checked
if the syntax highlighting pref is enabled.
1) Enable syntax highlighting pref.
2) View Selection Source, see that the page's syntax is highlighted.
3) View -> Syntax Highlighting, see that the menuitem isn't checked even though
the page's syntax is highlighted.
|
Reporter | |
Comment 1•23 years ago
|
||
|
||
Comment 2•23 years ago
|
||
Comment on attachment 93894 [details] [diff] [review]
patch
sr=bzbarsky
Attachment #93894 -
Flags: superreview+
For global consistency, you might unlink the viewsource.css here:
var wrapClass = gWrapLongLines ? ' class="wrap"' : '';
var source =
'<html>'
+ '<head><title>Mozilla</title>'
- + '<link rel="stylesheet" type="text/css" href="' + gViewSourceCSS + '">'
+ + gHighlightSyntax ?
+ '<link rel="stylesheet" type="text/css" href="' + gViewSourceCSS + '">'
+ : ''
+ '<style type="text/css">'
|
Assignee | |
Comment 4•23 years ago
|
||
Comment on attachment 93894 [details] [diff] [review]
patch
Please either
1) Use cmd_hightlightSyntax as per viewSource.js
2) Now that bug 96229 is fixed remove the use of cmd_hightlightSyntax from both
files. Note that during the command event the checked state of the menuitem is
now guaranteed to be the opposite of the saved checked state of the command,
thus saving you from having to flip it manually as I was forced to for my
workaround.
Attachment #93894 -
Flags: review-
neil, interested in picking this up and doing a cleanup of the earlier workaround?
|
|
Assignee | |
Comment 6•23 years ago
|
||
|
|
Assignee | |
Updated•23 years ago
|
Attachment #93894 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 7•23 years ago
|
||
Comment on attachment 110769 [details] [diff] [review]
Proposed patch
bz, I'll leave the choice of reviewer up to you.
Attachment #110769 -
Flags: superreview?(bzbarsky)
|
|
||
Updated•23 years ago
|
Attachment #110769 -
Flags: superreview?(bzbarsky)
Attachment #110769 -
Flags: superreview+
Attachment #110769 -
Flags: review?(doron)
Although it is often (but not always) the case to stick to just one thing per
bug, I wouldn't mind, in this instance, a clean up of cmd_wrapLongLines as well
while here? For the good of the code.
It will look puzzling for a new person reading the code to see that highlight
and wraplonglines are coded so differently, and that wouldn't help the cause of
XUL which is often accused promptly.
|
||
Comment 9•23 years ago
|
||
Comment on attachment 110769 [details] [diff] [review]
Proposed patch
looks good. Perhaps add a () around the == part of:
+ var highlightSyntax = highlightSyntaxMenu.getAttribute("checked") == "true";
to make the code more readable.
Attachment #110769 -
Flags: review?(doron) → review+
|
|
Assignee | |
Comment 11•23 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
|
||
Updated•21 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•