Closed Bug 148695 Opened 19 years ago Closed 4 years ago
[PATCH] Implement protocol filter in JS Console
HJ asked me to file this bug while he's away, again. HJ e-mailed Joe Hewitt about this small JS Console enhancement and Joe liked HJ's idea. I'm about to attach a screenshot and a diff -u for this enhancement. Feel free to ask any question you might have and I'll try to contact HJ via the appropriate channels.
Proposed patch for this enhancement
CC'ed HJ (just to be sure)
Marking NEW so it gets loving for the patch.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Implement protocol filter in JS Console → [PATCH] Implement protocol filter in JS Console
So... from those menuitems it's not at all clear what the filter does (blocks out that protocol or only allows that protocol). > - if (!this.showChromeErrors && scriptError.sourceName.substr(0, 9) == "chrome://") > + > + if (!this.showChromeErrors && scriptError.sourceName.substr(0, 6) == "chrome") Why this change? That seems wrong to me... What about a future "chromefoo:" protocol? And please do not remove the license from consolebindings.xml even if you dislike it... ;)
Oh, and I think Jesse missed that this is pretty orthogonal to the chrome errors thing... (and in fact continues to support that). Though I would remove the "chrome" menuitem entirely rather than disabling it....
bz: how is this rfe orthogonal to the pref for showing chrome errors? That's the only reason I can imagine for using these menu items.
Jesse, that's a failing of your imagination. I would use these if I'm doing web browsing and debugging of a site at a file:// url in parallel (a quite common situation). In that case I would want to see only errors from file:// urls.
I don't see anything in the code that actually does anything... > gConsole._showChromeErrors = (showChromeErrors == true); If you're always going to read the pref then either get consoleBindings.xml to do it and read the value from there, or stop consoleBindings.xml from doing it and change it to a simple field. Either way you don't need your own local variable, and you don't need that silly comparison. And you don't have any access keys for your submenu items.
"So... from those menuitems it's not at all clear what the filter does (blocks out that protocol or only allows that protocol)." Ok, but how do we solve this issue? Any suggestions? "Why this change? That seems wrong to me... What about a future "chromefoo:" protocol?" Yeah, that is no good. I will make a new patch with the original. "And please do not remove the license from consolebindings.xml even if you dislike it... ;)" There is no license info so I only added one :-) http://lxr.mozilla.org/seamonkey/source/xpfe/components/console/resources/content/consoleBindings.xml "I don't see anything in the code that actually does anything..." But it works for you, right? "If you're always going to read the pref..." But that pref is only read once, when you open the menu, just to make sure you didn't change the pref in the preference menu, after starting the JS console. <snap:console.xul> <menupopup onpopupshowing="checkChromePref()"> </snap:console.xul> <snap:consoleBindings.xml> if (this._showChromeErrors != -1) return this._showChromeErrors; </snap:consoleBindings.xml> "...and you don't need that silly comparison" Hey, I just love it, like Jag does. Remember this: "Write what you mean" :-)
Well I don't apply patches without some assurance that they will work. Sorry that I missed the control flow on that showChromeErrors code, but the silly comparison is actually == true, not == -1.
gConsole._showChromeErrors = (showChromeErrors == true); This line is added this way because |gConsole._showChromeErrors| need to be set to true or false, but it is set to -1 at startup. "And you don't have any access keys for your submenu items." Someone (MPT) told me not to add access keys for radio type menu items, so I didn't.
>gConsole._showChromeErrors = (showChromeErrors == true); > >This line is added this way because |gConsole._showChromeErrors| need to be set >to true or false, but it is set to -1 at startup. but it's showChromeErrors that you are comparing to true... sigh... >>"And you don't have any access keys for your submenu items." > Someone (MPT) told me not to add access keys for radio type menu items, so I didn't. /me needs to mumble at mpt :-(
Well I can't understand why your variable doesn't hold either true or false as those are the only two values that getBoolPref can return. When it doesn't hold either true or false what's does about:config display?
I made a new update some time ago, but was unable to reply to this bug report because of me being away, job related stuff. Here are two dums of my latest modifications, this update will clear things up, I hope. http://multizilla.mozdev.org/screenshots/bughelper/bug-help-protocol-filter.jpg http://multizilla.mozdev.org/screenshots/bughelper/bug-help-view-source.jpg Btw, the 'View Source' menu items are disabled for items without links.
Status: NEW → ASSIGNED
Reassigning to me...updating Hardware/OS settings/
Assignee: hewitt → bugs4hj
Status: ASSIGNED → NEW
OS: Windows 2000 → All
Hardware: PC → All
Please use checkboxes instead of radio buttons. Radio buttons give me no way of saying I'm interested in http and https, but not chrome.
what's the status here? there's a console filter extension available now: http://forums.mozillazine.org/viewtopic.php?t=264146 perhaps a filter is the right way to go. This way you could just type "chrome://" to get all chrome messages
Assignee: bugs4hj → nobody
Product: SeaMonkey → Toolkit
QA Contact: jrgmorrison → error.console
The Error Console has been removed in favor of the Browser Console (see Bug 1278368), and the component is going to be removed. If this bug is also relevant in the Browser Console, please reopen and move this into Firefox -> Developer Tools: Console.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
I am mass-reopening and re-componenting every single one of the Toolkit:Error Console bugs that appear to have been closed without anyone even *glancing* at whether they were relevant to the Browser Console. If you want to close a whole bunch of old bugs -- FOR ANY REASON -- it is YOUR RESPONSIBILITY to check EVERY SINGLE ONE OF THEM and make sure they are no longer valid. Do not push that work onto the bug reporters. (It's okay to close bugs that haven't been touched in years when they don't have enough information for you to figure out whether the problem is still relevant to the current software - the reporter probably isn't coming back to clarify. But that is the ONLY situation where it is okay.) (I'm going to have to do this in two steps because of the way the "change several bugs at once" form works. Apologies for the extra bugspam.)
Status: RESOLVED → REOPENED
Component: Error Console → Developer Tools: Console
Product: Toolkit → Firefox
Resolution: WONTFIX → ---
The Error Console feature was removed entirely from the tree in Bug 1278368 and the bugzilla component is now being removed. We’ve migrated bugs that seem to also affect the Browser Console into the devtools component, please move this over if it was missed.
Status: REOPENED → RESOLVED
Closed: 4 years ago → 4 years ago
Component: Developer Tools: Console → Error Console
Product: Firefox → Toolkit
Resolution: --- → WONTFIX
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.