Last Comment Bug 465303 - Add Preference UI for accessibility.blockautorefresh and for Hardware Acceleration.
: Add Preference UI for accessibility.blockautorefresh and for Hardware Acceler...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: seamonkey2.1b2
Assigned To: Philip Chee
:
:
Mentors:
Depends on: 83265 558995
Blocks: 614591 625318
  Show dependency treegraph
 
Reported: 2008-11-17 01:13 PST by Stefan A. Möller
Modified: 2011-01-13 02:16 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 Add checkboxes to the Content Panel. (8.60 KB, patch)
2010-11-20 01:41 PST, Philip Chee
neil: review-
Details | Diff | Splinter Review
Screenshot of the Content Panel. (29.10 KB, image/png)
2010-11-20 01:45 PST, Philip Chee
neil: ui‑review+
Details
Patch v1.1 Non Instant-Apply. (9.51 KB, patch)
2010-11-20 12:25 PST, Philip Chee
no flags Details | Diff | Splinter Review
Patch v1.2 Fix Nits. (9.45 KB, patch)
2010-11-21 09:01 PST, Philip Chee
neil: review+
neil: superreview+
Details | Diff | Splinter Review
Patch v1.2a As checked-in (9.46 KB, patch)
2010-11-21 20:11 PST, Philip Chee
no flags Details | Diff | Splinter Review

Description Stefan A. Möller 2008-11-17 01:13:52 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081116 SeaMonkey/2.0a2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081116 SeaMonkey/2.0a2pre

This is the SeaMonkey-only version of bug 83265. Although bug 83265 is labelled as a core bug and has been marked fixed on 2007-02-08, it still does not apply to the latest SeaMonkey trunk build (2.0a2pre). The preference accessibility.blockautorefresh is missing.

Requesting this feature for SeaMonkey also, since Firefox 3.0 already has it.

Reproducible: Always
Comment 1 therube 2009-09-20 11:43:17 PDT
I was told, "The front end of Bug 83265 does not seem to have been ported to SeaMonkey" ... "the onRefreshAttempted listeners in the seamonkey browser code".

http://www.hirsch.sth.ac.at/~robert/thebot-logs/%23seamonkey-20090919-110005.xml

(META refresh)
Comment 2 therube 2010-05-06 21:59:10 PDT
accessibility.blockautorefresh looks to be working in a current Trunk.

Test page: http://www.searchtools.com/test/redirect/

Test case from Bug 83265 : https://bugzilla.mozilla.org/attachment.cgi?id=253431


With accessibility.blockautorefresh set to True, you are presented with a dialog, "SeaMonkey prevented this page from automatically redirecting to another page".


KB: http://kb.mozillazine.org/Accessibility.blockautorefresh


Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100502 SeaMonkey/2.1a1pre


So this should be marked as ... WONTFIX (for 2.0 branch).
Or is it ... FIXED cause it works on the 2.1a Trunk?

(META Refresh Redirect)
Comment 3 Jens Hatlak (:InvisibleSmiley) 2010-05-23 17:57:03 PDT
accessibility.blockautorefresh was ported in Bug 558995 (past SM 2.0) so this is currently trunk-only. Looking at Bug 558995 comment 24 suggests that this bug is still relevant, and it really is because FF Bug 83265 not only introduced accessibility.blockautorefresh itself but also a Preferences checkbox for it (in FF's advanced.xul). Thus I'm changing this bug to cover that part. Finally a Help bug should be created once this bug is done. For now: confirming. FF has:

<preference id="accessibility.blockautorefresh"
            name="accessibility.blockautorefresh"
            type="bool"/>

<checkbox id="blockAutoRefresh"
          label="&blockAutoRefresh.label;"
          accesskey="&blockAutoRefresh.accesskey;"
          preference="accessibility.blockautorefresh"/>

<!ENTITY blockAutoRefresh.label     "Warn me when web sites try to redirect or reload the page">
<!ENTITY blockAutoRefresh.accesskey "r">

I think this could either go under Appearance/Content or Advanced/Scripts & Plugins. Since it's not really a script or plugin and the former category already has a list of three individual checkboxes I suggest to use that one.

BTW: We seem to be missing support for accessibility.browsewithcaret ("Always use the cursor keys to navigate within pages") completely. Do we have a bug for that?
Comment 4 Jens Hatlak (:InvisibleSmiley) 2010-05-26 10:59:46 PDT
(In reply to comment #3)
> BTW: We seem to be missing support for accessibility.browsewithcaret ("Always
> use the cursor keys to navigate within pages") completely.

Well, not completely. I found that we support the pref (for an eternity already!), we just have no checkbox in Preferences for it. Filed bug 568283 to investigate that. Essentially it's just F7-by-default mode.
Comment 5 Philip Chee 2010-11-20 01:38:16 PST
Add Preference UI for accessibility.blockautorefresh and for Hardware Acceleration.

I was looking into this bug as well as porting the following bugs:

Create an easy way to toggle the state of hardware acceleration:
  Bug 587857 Add a pref on Windows to easily disable Direct2D.
  Bug 594173 "Advanced > Use hardware acceleration when available" needs to control layers.accelerate-none pref

Since they all go into the Content panel I thought for matters of efficiency of doing both in one patch. So I'm re-purposing this bug slightly and making the summary a bit less verbose.
Comment 6 Philip Chee 2010-11-20 01:41:17 PST
Created attachment 492045 [details] [diff] [review]
Patch v1.0 Add checkboxes to the Content Panel.

Make it so.
Comment 7 Philip Chee 2010-11-20 01:45:52 PST
Created attachment 492046 [details]
Screenshot of the Content Panel.
Comment 8 neil@parkwaycc.co.uk 2010-11-20 07:48:10 PST
Comment on attachment 492045 [details] [diff] [review]
Patch v1.0 Add checkboxes to the Content Panel.

>+  if (/^Win/.test(navigator.platform)) {
>+    var pref = document.getElementById("layers.accelerate-none");
>+    Components.classes["@mozilla.org/preferences-service;1"]
>+              .getService(Components.interfaces.nsIPrefBranch2)
>+              .setBoolPref("gfx.direct2d.disabled", !pref.value);
This is wrong because it sets the preference immediately, but Windows is ironically the one platform where instantApply defaults to false...
Comment 9 Philip Chee 2010-11-20 12:25:16 PST
Created attachment 492081 [details] [diff] [review]
Patch v1.1 Non Instant-Apply.

> >+  if (/^Win/.test(navigator.platform)) {
> >+    var pref = document.getElementById("layers.accelerate-none");
> >+    Components.classes["@mozilla.org/preferences-service;1"]
> >+              .getService(Components.interfaces.nsIPrefBranch2)
> >+              .setBoolPref("gfx.direct2d.disabled", !pref.value);
> This is wrong because it sets the preference immediately, but Windows is
> ironically the one platform where instantApply defaults to false...

Updating the <preference> element seems to work.
Comment 10 Philip Chee 2010-11-20 12:40:46 PST
> This is wrong because it sets the preference immediately, but Windows is
> ironically the one platform where instantApply defaults to false...

Of course "gfx.direct2d.disabled" is only rescanned on startup so I guess the Firefox developers took a short cut on this.
Comment 11 neil@parkwaycc.co.uk 2010-11-20 12:57:02 PST
(In reply to comment #10)
> > This is wrong because it sets the preference immediately, but Windows is
> > ironically the one platform where instantApply defaults to false...
> 
> Of course "gfx.direct2d.disabled" is only rescanned on startup so I guess the
> Firefox developers took a short cut on this.

How is that relevant? If you change the checkbox and click cancel then the direct2d pref still gets changed...
Comment 12 neil@parkwaycc.co.uk 2010-11-20 13:08:44 PST
Comment on attachment 492081 [details] [diff] [review]
Patch v1.1 Non Instant-Apply.

>+    var accel = document.getElementById("layers.accelerate-none");
>+    document.getElementById("gfx.direct2d.disabled").value = !accel.value;
Is the ! because of the inverted="true"?
(Might be worth inverting the other pref to reduce confusion.)

>+                  onchange="updateHardwareAcceleration();"/>
During the event, this is the preference element. So it might be possible to use this (pun intended) to save on some code.

>                   type="bool" inverted="true"/>
pref-tabs.xul is the odd one out here.
Comment 13 Philip Chee 2010-11-21 09:01:58 PST
Created attachment 492188 [details] [diff] [review]
Patch v1.2 Fix Nits.

> >+    var accel = document.getElementById("layers.accelerate-none");
> >+    document.getElementById("gfx.direct2d.disabled").value = !accel.value;
> Is the ! because of the inverted="true"?
Yes.

> (Might be worth inverting the other pref to reduce confusion.)
I thought a bit on that.

Against: Doesn't make sense since there is no UI for it. Also saves a few bytes.
For: Reduce confusion.

In the end I flipped a coin. But I don't have strong opinions either way so I'll flip the pref.

> >+                  onchange="updateHardwareAcceleration();"/>
> During the event, this is the preference element. So it might be possible to
> use this (pun intended) to save on some code.
Fixed.

> >                   type="bool" inverted="true"/>
> pref-tabs.xul is the odd one out here.
O.I.C. Fixed.
Comment 14 neil@parkwaycc.co.uk 2010-11-21 09:23:57 PST
Comment on attachment 492188 [details] [diff] [review]
Patch v1.2 Fix Nits.

>+function updateHardwareAcceleration(that)
Nit: please pass value directly. r+sr=me with that fixed.
Comment 15 Philip Chee 2010-11-21 20:11:53 PST
Created attachment 492244 [details] [diff] [review]
Patch v1.2a As checked-in
Comment 16 Philip Chee 2010-11-21 20:15:55 PST
pushed to comm-central
http://hg.mozilla.org/comm-central/rev/dec9e1fed085
Comment 17 Stefan [:stefanh] 2010-11-22 00:07:47 PST
Is there a help bug for the prepane changes? If not, would you mind filing one?
Comment 18 Philip Chee 2010-11-24 10:25:42 PST
> Is there a help bug for the prepane changes? If not, would you mind filing one?
Oops sorry. Thanks Jens for filing it for me.

Note You need to log in before you can comment on or make changes to this bug.