[PATCH] Implement protocol filter in JS Console

RESOLVED WONTFIX

Status

Toolkit Graveyard
Error Console
--
enhancement
RESOLVED WONTFIX
15 years ago
10 months ago

People

(Reporter: Neil Pryde, Unassigned)

Tracking

Details

Attachments

(2 attachments)

(Reporter)

Description

15 years ago
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.
(Reporter)

Comment 1

15 years ago
Created attachment 85992 [details]
Screenshot of new menu in JS Console
(Reporter)

Comment 2

15 years ago
Created attachment 85996 [details] [diff] [review]
diff -u of files

Proposed patch for this enhancement
(Reporter)

Comment 3

15 years ago
CC'ed HJ (just to be sure)
(Reporter)

Comment 4

15 years ago
and setting some keywords (sorry for the SPAM)
Keywords: patch, review

Comment 5

15 years ago
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

Comment 6

15 years ago
There's already a checkbox in Debug prefs for "show javascript errors and 
warnings in mozilla chrome in the js console".  I this this is unnecessary.
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....

Comment 9

15 years ago
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.

Comment 11

15 years ago
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.

Comment 12

15 years ago
"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" :-)

Comment 13

15 years ago
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.

Comment 14

15 years ago
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.

Comment 15

15 years ago
>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 :-(

Comment 16

15 years ago
showChromeErrors is set to true at start but,

try {
   showChromeErrors = pref.getBoolPref("javascript.options.showInConsole");
} catch(ex) {}
dump("showChromeErrors = " + showChromeErrors);

this dump doesn't always display false or true, so what should I do?

Comment 17

15 years ago
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?

Comment 18

14 years ago
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

Comment 19

14 years ago
Reassigning to me...updating Hardware/OS settings/
Assignee: hewitt → bugs4hj
Status: ASSIGNED → NEW
OS: Windows 2000 → All
Hardware: PC → All

Comment 20

14 years ago
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.

Comment 21

12 years ago
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)

Updated

9 years ago
Product: Core → SeaMonkey

Updated

7 years ago
Assignee: bugs4hj → nobody
Component: Error Console → Error Console
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
Last Resolved: 10 months ago
Resolution: --- → WONTFIX

Comment 23

10 months ago
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
Last Resolved: 10 months ago10 months ago
Component: Developer Tools: Console → Error Console
Product: Firefox → Toolkit
Resolution: --- → WONTFIX
(Assignee)

Updated

10 months ago
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.