419609 - add hidden pref to disable site-specific zoom

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: implements hidden pref

Site-specific zoom will be a boon to many users, but some power users (and some existing users used to the old behavior) will find it more annoying than helpful, and it would be useful for there to be a way for those users to turn it off.

It would be useful for extension authors experimenting with innovative new applications of page zoom, too, like glazman with his recent Zoom It! extension.

This doesn't rise to the level of a visible pref but seems worth making a hidden pref.  Here's a patch that adds that.  Requesting wanted-firefox3 for this change.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

See also bug 419612 for an example of a kind of hybrid per-site/per-session approach that might be useful for some users but seems too complicated to implement in the Firefox 3 timeframe.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 305730 [details] [diff] [review]
patch v1: implements hidden pref

>Index: browser/base/content/browser-textZoom.js

>+  get _prefBranch FullZoom_get__prefBranch() {
>+    delete this._prefBranch;
>+    return this._prefBranch = Cc["@mozilla.org/preferences-service;1"].
>+                              getService(Ci.nsIPrefBranch).
>+                              QueryInterface(Ci.nsIPrefBranch2);

nsIPrefBranch2 inherits from nsIPrefBranch, so no need for the extra QI. Just pass nsIPrefBranch2 to getService().

Looks fine otherwise.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: addresses review nit

This patch is identical to the previous one except that the pref service getter QIs simply to nsIPrefBranch2.  This is the version of the patch I'll check in.

Requesting approval for this low-risk wanted bug that improves compatibility with Firefox 2, better serves our power users (via only hidden UI, nothing that affects regular users), and simplifies extensibility for extensions that want to experiment with innovations to the new page zoom functionality.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 305813 [details] [diff] [review]
patch v2: addresses review nit

a=beltzner for 1.9

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Checking in browser/base/content/browser-textZoom.js;
/cvsroot/mozilla/browser/base/content/browser-textZoom.js,v  <--  browser-textZoom.js
new revision: 1.9; previous revision: 1.8
done
Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.285; previous revision: 1.284
done

Comment 6

[Tony Chung [:tchung]]

myk, how can this fix be verified?

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #6)
> myk, how can this fix be verified?

Use about:config to set the hidden pref browser.zoom.siteSpecific to false.  Then open two pages from the same site in tabs.  Zoom in on one of the pages, then switch to the other page.  It should stay the same size.

If you set the preference back to true and then try the same thing, the second page should zoom to the level to which you set the first page.

Comment 8

[Mark C]

Is it worth changing this preference from a boolean to an integer? (see bug #425381 for my thoughts on this)

Having it as an integer would allow bug #419612 to be addressed, in part, by adding another allowed value to the browser.zoom.siteSpecific pref rather than by creating another new pref.