Add option to toggle JavaScript to Toolbox Options panel

RESOLVED FIXED in Firefox 24

Status

()

Firefox
Developer Tools: Framework
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jwalker, Assigned: miker)

Tracking

Trunk
Firefox 24
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

I think we should only actually disable JS while the toolbox is open, much as we're only planning on disabling the cache while the toolbox is open in bug 864098.
Duplicate of this bug: 873740

Comment 2

4 years ago
The option to toggle JavaScript should be available right in the Web Dev menu. I use it often because there are plenty of website on which JS makes the pages hard or almost impossible to use.
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
It will be a per tab setting in the toolbox options panel.

This setting will only apply until either the toolbox or the tab is closed.

Scratchpad, web console etc. should still be capable of running JS though.
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #3)
> It will be a per tab setting in the toolbox options panel.
> 
> This setting will only apply until either the toolbox or the tab is closed.
> 
> Scratchpad, web console etc. should still be capable of running JS though.

\o/

(What kind of sorcery is this ? )

Comment 5

4 years ago
Will there be an option to enable context menu and windows resiszing/zorder, so that we can control it without resorting to addons?
(In reply to Ho Yiu Yeung from comment #5)
> Will there be an option to enable context menu and windows resiszing/zorder,
> so that we can control it without resorting to addons?

This bug won't provide those features. I'm not clear exactly what you're asking for, but perhaps it's best to raise new bugs, and describe what you asking for more fully in new bugs.
Thanks.
Created attachment 755865 [details] [diff] [review]
Patch

Until bug 409737 is fixed we need to refresh the page when this option is changed. With that in mind it works perfectly.

Try:
https://tbpl.mozilla.org/?tree=Try&rev=c6a13b2d71d3
Attachment #755865 - Flags: review?(jwalker)
Comment on attachment 755865 [details] [diff] [review]
Patch

Review of attachment 755865 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/framework/toolbox-options.js
@@ +142,5 @@
>  
> +  _addListeners: function() {
> +    let disableJSNode = this.panelDoc.getElementById("devtools-disable-javascript");
> +    disableJSNode.addEventListener("click", this._disableJSClicked, false);
> +  },

Nit: I was wondering if it was worth adding a function to encapsulate 2 lines, especially when the addEventListener makes sense of the bind on line 36?

@@ +172,5 @@
>    destroy: function OP_destroy() {
> +    let disableJSNode = this.panelDoc.getElementById("devtools-disable-javascript");
> +    disableJSNode.removeEventListener("click", this._disableJSClicked, false);
> +
> +    this.panelWin = this.panelDoc = this.toolbox = null;

this._disableJSClicked = null?
Probably not a memory leak though?

::: browser/devtools/framework/toolbox-options.xul
@@ +38,5 @@
>                      data-pref="devtools.debugger.remote-enabled"/>
> +          <checkbox id="devtools-disable-javascript"
> +                    label="&options.disableJavaScript.label;"
> +                    tooltiptext="&options.disableJavaScript.tooltip;"/>
> +          <label value="&options.context.requiresRestart;"/>

Do you mean requiresRestart here?

Also, I don't think we can rely on someone reading a tool-tip to understand that the state is temporary. Happy for second opinions, but I think we need to make this clear.
Attachment #755865 - Flags: review?(jwalker) → review+
Created attachment 756595 [details] [diff] [review]
Patch v2

(In reply to Joe Walker [:jwalker] from comment #8)
> Comment on attachment 755865 [details] [diff] [review]
> Patch
> 
> Review of attachment 755865 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/framework/toolbox-options.js
> @@ +142,5 @@
> >  
> > +  _addListeners: function() {
> > +    let disableJSNode = this.panelDoc.getElementById("devtools-disable-javascript");
> > +    disableJSNode.addEventListener("click", this._disableJSClicked, false);
> > +  },
> 
> Nit: I was wondering if it was worth adding a function to encapsulate 2
> lines, especially when the addEventListener makes sense of the bind on line
> 36?
> 

Agreed, moved into init().

> @@ +172,5 @@
> >    destroy: function OP_destroy() {
> > +    let disableJSNode = this.panelDoc.getElementById("devtools-disable-javascript");
> > +    disableJSNode.removeEventListener("click", this._disableJSClicked, false);
> > +
> > +    this.panelWin = this.panelDoc = this.toolbox = null;
> 
> this._disableJSClicked = null?
> Probably not a memory leak though?
> 

It wouldn't leak but I have added it to the destructor.

> ::: browser/devtools/framework/toolbox-options.xul
> @@ +38,5 @@
> >                      data-pref="devtools.debugger.remote-enabled"/>
> > +          <checkbox id="devtools-disable-javascript"
> > +                    label="&options.disableJavaScript.label;"
> > +                    tooltiptext="&options.disableJavaScript.tooltip;"/>
> > +          <label value="&options.context.requiresRestart;"/>
> 
> Do you mean requiresRestart here?
> 

Yes. It used to be part of the section heading but now options that require a restart have an * with a "* Requires browser restart" footnote.

> Also, I don't think we can rely on someone reading a tool-tip to understand
> that the state is temporary. Happy for second opinions, but I think we need
> to make this clear.

There is also a "† Triggers page refresh" footnote attached to this option and I have added (temporary) after the option.
Attachment #755865 - Attachment is obsolete: true

Comment 10

4 years ago
Disable images e JS removed?? That was just the stupidest idea ever had to Mozilla. These options have always been useful for the advanced user and NEVER been a problem for the average user. Why? Because the average user ignores these options. It does not change them. I have always been faithful to Firefox because unlike the others, it is (was?) Customizable. But it seems that in the rush to be a copy of Google Chrome it stop now. Oh ****! Put this in the "about: config" does not solve anything. To change the "about: config" I have to type and type. Are you mad? They are destroying my browser. Alex Limi you are a dunghill idiot!!

Comment 11

4 years ago
ALEX LIMI ... stop making **** ... **** ... **** ... with Firefox!!!
Whiteboard: [land-in-fx-team]
f1840662@rmqkr.net's account has been disabled. Abusing community members is not acceptable behaviour.

Gerv

Comment 13

4 years ago
All you and your stupid ideas go to s.h.i.t!! Alex Limi worked for Google, can only give this s.h.i.t on Mozilla!!! Mozilla, why not just sell your soul to Google??

Comment 14

4 years ago
Gervase, instead of acting like you've been offended, I suggest you try to understand what that user is saying. Many of us agree with him: you (collectively) have been ruining Firefox for us. 

Your attempts to save users from themselves are USELESS because only power users bother to change options. Regular users don't do that, and most don't even seem to be aware of them. So, while you are making an appearance of doing valuable work, you are actually making Firefox WORSE. One of main reasons I don't use Chrome and have recommended that others stay with Firefox is because Firefox is more customizable, but you have been taking this power away from us.

Instead of fixing real bugs (e.g., preventing Flash from stealing keystrokes, crashes, unresponsiveness, etc.), you have been removing features some of us have been accustomed to and find very valuable.
(In reply to Roman R. from comment #14)
> Gervase, instead of acting like you've been offended, I suggest you try to
> understand what that user is saying. Many of us agree with him: you
> (collectively) have been ruining Firefox for us. 
> 
> Your attempts to save users from themselves are USELESS because only power
> users bother to change options. Regular users don't do that, and most don't
> even seem to be aware of them. So, while you are making an appearance of
> doing valuable work, you are actually making Firefox WORSE. One of main
> reasons I don't use Chrome and have recommended that others stay with
> Firefox is because Firefox is more customizable, but you have been taking
> this power away from us.
> 
> Instead of fixing real bugs (e.g., preventing Flash from stealing
> keystrokes, crashes, unresponsiveness, etc.), you have been removing
> features some of us have been accustomed to and find very valuable.

Hi Roman, first of all, I asked Gerv to do something about the user. Its not his fault or anything. secondly. If you look at this bug, then we are *actually* trying to fix things here. Putting these kind of settings in their correct places, and making them user friendly and usable such that it does not break things (like it could do previously).

Moreover, let apart abusing one of Mozilla's employee (or in fact any person in general), he is comment is irrelevant to the bug. This is not the right bug to whine about the decision that has been made.

Please refrain yourself from further commenting here so as to make further irrelevant comments on this bug. Bugzilla is not a forum.
s/he is/his
Created attachment 759898 [details] [diff] [review]
rebased.

so, I wanted to base another bug on this one, then I found out that the web console's persistant logs patch bitrotted it. So rebased.
Attachment #756595 - Attachment is obsolete: true

Comment 18

4 years ago
Not sure if it's the right place (bug) but it seems in latest nightly NoScript: "Automatically reload affected pages when permissions change" turned to off (unchecked) doesn't make any effect - firefox refreshes page when javascript is allowed for a page even when You tell NoScript not to. So either NoScript dev needs to update (again) his add-on to meet "newest changes" or some (new?) code on the side of Firefox be corrected.

On the side note: I got used to about:config to change settings that got "taken away" from GUI, though it's harder to guide non-power users to, for example, disable images when they use network connections with bandwidth limits (GSM modem in most countries or still some aDSL lines - like St. Helena Island provider). Also would be nice to include per-site image permissions for FF Mobile for this reason. :)
https://hg.mozilla.org/integration/fx-team/rev/a4ddc0934d82
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/a4ddc0934d82
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 24

Comment 21

4 years ago
I am curious, this is marked as assigned, resolved, target FF 24, does this mean that the option box will be restored in Options/Content to check or uncheck Enable javascript, along with the associated three advanced options?
No, this bug is not related to the previous option that was available under Options -> Content in the general FF options.  There was a recent discussion[1] about the removal of that option on the firefox-dev mailing list.  If you have feedback about that, the mailing list is the best place to convey it.

The option in this bug is exposed for web developers to use temporarily to debug a given page.  If you open the toolbox by going to the Tools menu -> Web Developer -> Toggle Tools, you can access it by going to the Developer Tools options by clicking the gear icon in the top-left of the toolbox.

[1]: https://groups.google.com/d/topic/firefox-dev/VQ4xshKgDrg/discussion
You need to log in before you can comment on or make changes to this bug.