77730 - JavaScript on/off indicator in Error Console

Status: RESOLVED WONTFIX

(Core Graveyard :: Error Console, enhancement)

Description

[Alexey Chernyak]

This RFE is about reflecting JavaScript options which are currently located at
Preferences -> Advanced, inside JavaScript Console.

1.
We need an indication of wether JavaScript is on or off in JS Console. This can
be reflected in Console title. Or when JS is off, just print a message in the
Console saying so.
At the moment if someone disables JavaScript in prefs, it's hard to figure out
why it's not working when HTML page doesn't have <noscript> tag.
Indication of it in JS Console would be of great help.

2.
Provide means of turning JS on or off in JS Console menu.
In the least, this can be done with a link under Edit menu which opens
Preferences -> Advanced.
Or, ideally, an On/Off menu switch.
At the moment to do that have to go back to Browser window, then to Prefs, then
to Advanced and switch it there. This takes too long.

Also might consider moving out JS options from Preferences alltogether and
keeping them in JS Console?

Note:
Indicator/Notifier, I think, should reflect Browser JS status, not Mail/News JS.
Unless can think of a non-confusing way to do both.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

taking, I have a patch which adds a checkbox besides the "Clear" button - this
is both an indicator and a way to turn JS off/on.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch with accesskey

Comment 4

[timeless]

Comment on attachment 78622 [details] [diff] [review]
patch with accesskey

when you check it in, please use "J" instead of "j"

Comment 5

[Alexey Chernyak]

I was actually thinking about a menu being inserted in between View and Tools
which would contain enable/disable checkbox switches for the following preferences:

JavaScript in Navigator
JavaScript in Mail & News (possibly with a security warning?)
-----
Show Strict Warnings
Show Errors in Mozilla chrome

And any other JS-related prefs. And then possibly removing some or all of them
from the Prefs dialog?
But this is still very good and better than nothing. :o)

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

I used the checkbox in the toolbar because it is IMHO easier to access there,
there's enough space and it can easily serve as the indicator you want.

> And then possibly removing some or all of them
> from the Prefs dialog?

I'm unable to find JS Console preferences in the Pref Dialog - can you tell me
where they are?

Comment 7

[Alexey Chernyak]

Oh the ease of access is not an issue here. Turning JavaScript on and off isn't
something you would do on a regular basis. You would do it to test something,
and do it very rarely.
The issue is, when you turned off JS during testing few days ago and forgot
about it, you try to debug another script and it doesn't work. Console is the
first place you look to see if there are any errors. And when there are none,
yet the script is not working, you start wasting time debugging it, thinking
that you screwed up with script's logic somewhere. That is until you realise the
JS is switched off.

It's not as much a need for being able to turn JS on and off quickly, but rather
a need for being able to find the option more quickly. And putting it into JS
console menu would serve the purpose very well. "No erros? Why it's not working
then? Oh! Familiar menu! I was switching off things in here yesterday! Silly me!"

The JS options in Prefs dialog can be found under 2 panes:

Advanced->Scripts & Windows
Debug

I'm not aware if there are any more. Rather hard to find aren't they? So yeah, a
menu in JS Console would simplify things greatly.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

I don't quite agree. JS Console needs no way for setting MailNews' JS Enabled
state, because that's set far too seldom. The checkbox can serve both as a
indicator that you want anyway and as a way for toggling the state.

The other two prefs you mention (Show Strict Warnings, Show Errors in Mozilla
chrome), on the other hand, should probably be part of JS Console menus and be
removed from the preferences.

I'll work on a patch to do this.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: new patch

Also adds the two preferences to the View Menu because adding a new menu for
just two items is unnecessary imho.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: better patch

OK, so if I understand you correctly, this should be what you want:
To the right of the clear button is now a text reading "Javascript is
{enabled|disabled}", and there are the three prefs in the View Menu. (if any of
these settings is changed while js console is open, the pref indicators are
updated).

Does this satisfy you now? :)

Comment 11

[Alexey Chernyak]

Sounds grrrrrrrreat as long as the text is clearly separated from the toolbar
buttons and can't be confused as one of them and doesn't get in the way.
Might even float it to the right side (put a spring between it and Clear button?)
Would have to consider internationalisation issues of RTL languages here I
guess, dnno I'm not an expert on those. ;o)

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, I've now moved the text to the right of the dialog
will attach patch in a second

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: latest patch

Comment 14

[Alexey Chernyak]

Can you please attach screenshot as well if possible

Comment 15

[Henrik Gemal]

could you attach screenshot?

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: screenshot

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: screenshot

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 87565 [details]
screenshot

sorry, couldn't screenshot the opened menu (gimp wouldn't let me)

Comment 19

[Alexey Chernyak]

It looks kinnda weird and unusual to me.
But hey, it's something!
I'm sure mpt would get medieval on our arses for this, should we summon him? ;o)

Put a toolbarseparator between spacer and label?
What would it look like if the text was of disabled colour?

And how about removing JS stuff from the Prefs in this patch as well?
Doing all in one go.

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: Remove prefs from Debug panel

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

btw, the bottom of the view menu looks like this with my patch:
--------------------------------
Enable _Javascript
Enable _Strict Warnings
Show _Chrome Errors and Warnings

Comment 22

[Matthew T (active 1999-2002)]

In my current semi-conscious state I don't find the screenshot to be 
particularly bad, though I'm sure people will click on the `JavaScript is 
enabled' and wonder why nothing happens. However, that is a theme bug with the 
*rest* of the buttons (they don't look like buttons), not a bug with 
Christian's fix.

The menu items are a bit unwieldy, however, and they begin with `Enable' which 
is redundant with the fact that they're checkbox items. I suggest:
*   / Enable _Javascript               >    / Run Scripts
*   / Enable Strict Warnings           >    / Include Strict Warnings

(Bonus points for changing the equivalent checkbox in the Preferences dialog to 
`Run scripts in Web pages'.)

As for `Show _Chrome Errors and Warnings', I find it mildly appalling that 
you're trying to include this in the menus. It should be in the Debug panel of 
Preferences, and nowhere else.

Comment 23

[Alexey Chernyak]

I just noticed that the Debug Panel itsel has been removed from Preferences in
Mozilla 1.0
No way of activating Strict Warnings? Oh the humanity!
Unless they are enabled by default ofcoarse. Which I reckon they should be, with
that option removed, unless they cause a significant slowdown of some sort to
Mozilla's work. The "All", "Errors", "Warnings" buttons in JS Console replace
this option quite well.

Anyways, mpt, I don't think changing "Enable JavaScript" to "Run JavaScript" will
make it easy for a person who is searching how to disable JavaScript execution
to find it.
I think it should stay as "Enable JavaScript" as this is the wording of this
option users are familiar with across browsers and the one they will look for.
However View menu is not the ideal place for it.

I agree on the rest of your points. Though instead of "Include Strict Warnings",
"Show Strict Warnings" might be a better alternative.

Comment 24

[Alexey Chernyak]

On the second thought, with "Include Strict Warnings" or "Show Strict Warnings"
the "Warnings" button doesn't make much sense. While with "Enable Strict
Warnings" it does.

And once again, View menu is not the best place for "Enable Strict Warnings".
However these options should *definately* be somewhere in the menu of JS Console.

Comment 25

[Christian :Biesinger (don't email me, ping me on IRC)]

I can imagine that there are warnings that are not strict warnings (like
reference to undefined property, which I would assume to always be printed). So
a warnings button still makes sense.

And I think View is perfectly suited, because View is the menu controlling the
View, ie. what's in the console, so Chrome Warnings and Strict Warnings
definitely belong in there. I don't think it's worth creating a menu for it.

mpt, on second thought, I think that a statusbar might be a better place for the
information whether JS is enabled or not. Would putting that into a newly
created statusbar be OK with you? (do you want a screenshot of that?)

As for chrome errors and warnings, I think it makes sense putting it into JS
Console, because these are enabled by default and are possibly annoying to web
developers (the target audience of JS Console, who else looks there?), and they
don't want to drown in Mozilla's own errors and warnings, which occur a lot esp.
if strict warnings are on, something web developers might want for their pages.

Comment 26

[Henrik Gemal]

any chance for a pref for "show *only* chrome errors/warnings"

Comment 27

[Matthew T (active 1999-2002)]

To make it clearer than I thought was necessary: The reason the option for 
chrome warnings should be in the Debug panel isn't because a menu would be a 
bad place for it if the option existed. It's because the option shouldn't exist 
*at all* in release builds. It's useful only for people debugging Mozilla's 
front end, not for users, and having a menu item for it would be extremely lame 
and unprofessional.

So, Christian, if you're going for a thorough fix-up rather than a shallow
fix-up (thankyou!), here's a shortlist:
(1) remove the toolbar;
(2) replace it with checkbox items in the `View' menu
    |
    | / Run Web Scripts
    | ----------------------------
    | / Errors
    | / Messages
    | / Warnings
    | / Include Strict Warnings
    | ----------------------------
    (where the `Include Strict Warnings' item is only enabled if the `Warnings'
    item is checked);
(3) rename the pref in the Preferences dialog to say `Run scripts in Web pages'
    (where the `Run Web Scripts' menu item in the JS Console is an abbreviation
    of this);
(4) add a status bar in the JS Console reporting on the state of the pref.

Comment 28

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: preferences part of the patch

remove the strict pref from the debug panel, change wording of enable
javascript

Comment 29

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: Screenshot of new look of JS Console

(also marking old patch obsolete, since a new one will come soon)

This is the new look of JS Console. Also shows the View Menu.

Comment 30

[Matthew T (active 1999-2002)]

Comment on attachment 89918 [details]
Screenshot of new look of JS Console

(1) `All' should not be present.
(2) `Errors', `Messages', and `Warnings' should be checkbox items, not radio
items.
(3) `Messages' should come before `Warnings', so that `Include Strict Warnings'
immediately follows the `Warnings' item on which it depends.
(4) `Errors' should have the accesskey `E'.
(5) `Messages' should have the accesskey `M'.
(6) `Warnings' should have the accesskey `W'.

Other than that, looks good ... :-)

Comment 31

[Alexey Chernyak]

Looking at that screenshot I once again can't help but feel that "Run Web
Scripts" and "Include Strict Warnings" have no place under View menu.
They will only confuse users.

From a user point of view I can't immagine what the difference between
"Warnings" and "Include Strict Warnings" can be.

If you still want to leave them there, I think it's better to rename "Include
Strict Warnings" to "Process Strict Warnings" and move it up next to "Run Web
Scripts".

Comment 32

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: new screenshot

screenshot of latest version. now uses checkboxes.

alexey, I'll let mpt comment about that; I personally really want strict
warnings to stay in the menu (or another one)

Comment 33

[timeless]

mpt was embarassed when i pointed out [view>run web scripts], so there's a bit 
more work to be done

Comment 34

[Alexey Chernyak]

I definately want both of those settings in Console menu as well. But View menu
is just wrong for them! Especially for enabling strict warnings processing.

also mpt:
>    (where the `Include Strict Warnings' item is only enabled if the `Warnings'
>    item is checked);

This dependancy I believe should be the other way around.

Comment 35

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: console part of patch

because mpt (mozilla's ui expert) insisted on Include Strict Warnings to be
named like that and be placed in the View menu, I left that as-is.

Comment 36

[timeless]

Comment on attachment 89932 [details] [diff] [review]
console part of patch

the one change we talked about on irc, and clarification from mpt about clear.

Comment 37

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: final (?) patch

(attachment 89914 [details] [diff] [review] is also needed)

Comment 38

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 89940 [details] [diff] [review]
final (?) patch

marking timeless's r=

Comment 39

[Christian :Biesinger (don't email me, ping me on IRC)]

1.1b is out...

Comment 40

[jag (Peter Annema)]

Can we get a screenshot of what it looks like now?

rginda, are you as module owner okay with these changes?

Comment 41

[Christian :Biesinger (don't email me, ping me on IRC)]

oh, rginda is the module owner? I thought hewitt was...

sure, I'll create a screenshot in a second

Comment 42

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: screenshot of js console with patch, View menu open

Comment 43

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: screenshot of edit menu

Comment 44

[Alexey Chernyak]

Me and mpt have agreed on IRC that above menu layout spec is totally
inappropriate for the functions that these menu items perform.

The spec needs to be redesigned.

Comment 45

[Christian :Biesinger (don't email me, ping me on IRC)]

ok guys. can you, at least one time, create a finished spec and only _then_ let
someone implement it?

Comment 46

[Matthew T (active 1999-2002)]

The screenshots look great. Excellent work, Christian.

Comment 47

[Robert Ginda]

hewitt is module owner here, jsconsole != jsdebugger.

Comment 48

[Joe Hewitt (gone)]

Silly rabbit, don't remove the toolbar!!! I use that all the time while debugging!

Comment 49

[Christian :Biesinger (don't email me, ping me on IRC)]

hewitt, well, you still have the options in the menu that now are in the toolbar...

if you really want me to I can of course add the toolbar back, I suppose...

Comment 50

[jag (Peter Annema)]

rginda, hewitt: oops

hewitt: the toolbar, if kept, would have to contain checkbox or toggle buttons
to show individual on/off display states (see attachment 92878 [details] and comment 30)
for "Errors", "Messages", "Warnings". That seems rather ugly (but maybe it
isn't?), so I then thought about a menulist which allows one to toggle the
states, but that's then essentially the same as the menu. The only thing left
would be "clear", which doesn't justify a toolbar by itself, so (apparently)
it's under the File menu now.

I can see where a toolbar would still be a good thing if you need to change this
stuff a lot while developing, so perhaps you could sketch us how you're
currently using the toolbar so we (well, they ;-) can take that into consideration?

What would make the js console a lot better for me is if I could delete (clear)
individual items and ranges, and/or if I could filter on site vs chrome. But
that's another bug (or two).

Comment 51

[Christian :Biesinger (don't email me, ping me on IRC)]

I have learned that UI patches are not wanted in this project. reassigning to
default owner.

Comment 52

[Sören 'Chucker' Kuklau (gone)]

hewitt: Checkboxes do not belong on a toolbar.

Comment 53

[Mike Kaply [:mkaply]]

Is this ever actually going to be done? Or can we close as WONTFIX

Comment 54

[Philip Chee]

I vote WONTFIX, this seems more appropriate to an extension if at all.

Comment 55

[Robert Kaiser]

Yes, I think that's great extension fodder.