Closed Bug 419609 Opened 17 years ago Closed 17 years ago

add hidden pref to disable site-specific zoom

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 beta4

People

(Reporter: myk, Assigned: myk)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch v1: implements hidden pref (obsolete) (deleted) — Splinter Review
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.
Flags: blocking-firefox3?
Attachment #305730 - Flags: review?(gavin.sharp)
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.
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
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.
Attachment #305730 - Flags: review?(gavin.sharp) → review+
Attached patch patch v2: addresses review nit (deleted) — Splinter Review
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.
Attachment #305730 - Attachment is obsolete: true
Attachment #305813 - Flags: approval1.9?
Comment on attachment 305813 [details] [diff] [review] patch v2: addresses review nit a=beltzner for 1.9
Attachment #305813 - Flags: approval1.9? → approval1.9+
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta4
myk, how can this fix be verified?
(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.
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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: