Closed
Bug 664788
Opened 14 years ago
Closed 14 years ago
console.log(message) doesn't show leading whitespace in message
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 7
People
(Reporter: ianbicking, Assigned: past)
References
Details
(Whiteboard: [fixed-in-devtools])
Attachments
(1 file, 2 obsolete files)
1.63 KB,
patch
|
msucan
:
review+
dao
:
review+
|
Details | Diff | Splinter Review |
I sometimes want to do:
console.log('Some stuff:');
console.log(' example:', foo);
console.log(' also:', foo2);
But the leading whitespace is not displayed. I wouldn't put those spaces there if I didn't want them to be there ;)
![]() |
||
Comment 1•14 years ago
|
||
IMHO, I think this is an extreme edge case. Most people won't care.
Assignee | ||
Comment 2•14 years ago
|
||
This has been bugging me as well, so I cooked up this patch that seems to fix it. It breaks lots of tests though that need fixing as well. Mihai do you have any less invasive ideas?
In my opinion, preserving spaces in the debug console is the right thing to do. Easily formatted debug strings can make output more useful and palatable. Sure, it's not really important, but I can't think of any down side.
Along the lines of Panos's patch, would it be easier just to change
aBody = aBody instanceof Ci.nsIDOMNode ?
aBody : aDocument.createTextNode(aBody);
to
aBody = aBody instanceof Ci.nsIDOMNode ?
aBody : aDocument.createTextNode(aBody.replace(/ /g, "\xa0"));
In my inexperience, I have no idea how this would affect any tests.
Comment 4•14 years ago
|
||
Comment on attachment 540050 [details] [diff] [review]
Patch v1
There must be a styling-based approach. Maybe:
https://developer.mozilla.org/en/CSS/white-space
Try white-space: pre-wrap to see if it works.
Attachment #540050 -
Flags: feedback?(mihai.sucan)
Assignee | ||
Comment 5•14 years ago
|
||
Bingo! Works as advertised, thanks!
Attachment #540050 -
Attachment is obsolete: true
Attachment #540440 -
Flags: review?(mihai.sucan)
Comment 6•14 years ago
|
||
Comment on attachment 540440 [details] [diff] [review]
Patch v2
Great to see it works!
Attachment #540440 -
Flags: review?(mihai.sucan) → review+
Assignee | ||
Comment 7•14 years ago
|
||
Comment on attachment 540440 [details] [diff] [review]
Patch v2
Thanks Mihai! Requesting peer review from dao.
Attachment #540440 -
Flags: review?(dao)
Comment 8•14 years ago
|
||
It's somewhat arguable whether that kind of rule belongs in the theme. Does the webConsole have a content stylesheet?
Comment 9•14 years ago
|
||
Comment on attachment 540440 [details] [diff] [review]
Patch v2
This belongs in content CSS. Unfortunately, the webconsole is mingled in the browser window's DOM rather than being a separate iframe.
Attachment #540440 -
Flags: review?(dao) → review-
Assignee | ||
Comment 10•14 years ago
|
||
I wasn't aware of the distinction, thanks. I guess that browser.css is the proper place, then?
Attachment #540440 -
Attachment is obsolete: true
Attachment #540707 -
Flags: review?(gavin.sharp)
Comment 11•14 years ago
|
||
Well, no, ideally there would be a webConsole.css somewhere in toolkit/components/console/, and ideally that stylesheet would be loaded in an iframe rather than browser.xul.
Comment 12•14 years ago
|
||
Comment on attachment 540707 [details] [diff] [review]
Patch v3
Indeed, this isn't really the proper place, but there are several things about the current web console code that aren't "proper". You could add a note to bug 635359, and also reference it in a comment here, but maybe it'd be cleaner to just add it to the theme for now and fix everything in bug 635359 (rather than expanding the set of things that need cleaning up to include browser.css).
Attachment #540707 -
Flags: review?(gavin.sharp) → review?(dao)
Comment 13•14 years ago
|
||
Comment on attachment 540440 [details] [diff] [review]
Patch v2
I'm still waiting for cleanup from the early days of the "heads up display" :/
Attachment #540440 -
Attachment is obsolete: false
Attachment #540440 -
Flags: review- → review+
Updated•14 years ago
|
Attachment #540707 -
Attachment is obsolete: true
Attachment #540707 -
Flags: review?(dao)
Assignee | ||
Comment 14•14 years ago
|
||
Thank you both. I added a comment to bug 635359 referencing comment 11 here.
Whiteboard: [land-in-devtools]
Updated•14 years ago
|
Whiteboard: [land-in-devtools] → [fixed-in-devtools]
Comment 15•14 years ago
|
||
Comment on attachment 540440 [details] [diff] [review]
Patch v2
http://hg.mozilla.org/projects/devtools/rev/fb02b664d02d
Comment 16•14 years ago
|
||
backed out to try to clear some orange:
http://hg.mozilla.org/projects/devtools/rev/9d2571f091aa
should be able to reland.
Whiteboard: [fixed-in-devtools] → [backed-out][land-in-devtools]
Comment 17•14 years ago
|
||
http://hg.mozilla.org/projects/devtools/rev/674f0bd87a13
http://hg.mozilla.org/mozilla-central/rev/674f0bd87a13
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [backed-out][land-in-devtools] → [fixed-in-devtools]
Updated•14 years ago
|
Target Milestone: --- → Firefox 7
Comment 18•14 years ago
|
||
Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110627 Firefox/7.0a1
Verified issue on Ubuntu 11.04, Mac OS X 10.6, WinXP, Win7 using the steps to reproduce from Comment 0.
Setting status to Verified Fixed.
Status: RESOLVED → VERIFIED
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•