401322 - missing "Text Size" zoom feature, Full page zoom should be an option not replacement

Status: VERIFIED FIXED

(Firefox :: General, enhancement, P4)

Description

[Marat Tanalin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; ru; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; ru; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8

As we can see in the latest trunk (Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007102605 Minefield/3.0a9pre), text size zoom is replaced with full page zoom, and there is no way to return usual text size zoom.

But for many people (including yours faithfully) full page zoom is completely useless and moreover UNUSABLE. It's even often is one of reasons for people to prefer Firefox to Opera -- they just want to increase text size -- not images, not layout at all, text only -- for more comfortable READING only, not for any another purposes.

Text size zoom is really advantageous distinction of Firefox from any other browser.

So, if full page zoom will be turned on by default then an option to return old behavior should exist. (Though, in my humble opinion, expediency of turning full page zoom by default is VERY questionable, and ZOOM should be an option [for some Opera switchers, etc.] -- and text size turned on by default as it was in previous Firefox versions).

Subject options, for example, may look like two radiobuttons in "View" -> "Zoom" menu:
- Zoom text only
- Zoom full page

with former option checked by default.

This may be duplicated in hierarchy of main Firefox settings if needed (perhaps someplace in "Tools" -> "Options" -> "Content"). Thanks.

Reproducible: Always

Steps to Reproduce:
Just try to resize text only -- you will get full page zoom instead.
Then search for option to return old behavior -- there is nothing ways to do it.
Actual Results:  
Text size zoom is replaced with full page zoom.

Expected Results:  
Full page zoom should be an option, not default and only way to zoom text.

Comment 1

[Ria Klaassen (not reading all bugmail)]

See Bug 401297. Mouse wheel zooming is still available. Or setting a minimum font-size.

Comment 2

[Marat Tanalin]

Hmm, Ria, what you meen with "still"? The topic is not about mouse wheel (or moreover minimum font size) at all.

Comment 3

[Marat Tanalin]

oops, errata: meen => mean

Comment 4

[Ria Klaassen (not reading all bugmail)]

The bug where this was implemented is bug 389628.

Comment 5

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #2)
> Hmm, Ria, what you meen with "still"?
I reacted to this line in comment 0: ".. and there is no way to return usual text size zoom". There is: mouse wheel scrolling.

> The topic is not about mouse wheel (or
> moreover minimum font size) at all.
>
A lot of people aren't aware of minimum size feature. Personally I have no opinion about the changes, as long as I can set a minimum size.  

Comment 6

[Marat Tanalin]

(In reply to comment #5)
Ria, CTRL + mouse wheel scrolling results in *full page zoom* for me. There is no evident way to resize text only now.

In my opinion, minimum font size feature is not whatever equivalent of text zoom, because unlike text zoom it hardly *limit* font size on all pages resulting in breaking initial proportions of text size in different text blocks on pages while these proportions is often method to accentuate different importance of text in different text blocks.

Comment 7

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #6)
> (In reply to comment #5)
> Ria, CTRL + mouse wheel scrolling results in *full page zoom* for me. There is
> no evident way to resize text only now.
You are right that the images are enlarged too, but it seems to zoom text in the same way (same steps) as Firefox 2.
I noticed that in branch you need to scroll down to enlarge, but in Minefield it is the opposite (scroll up). I think this is a separate bug (if not intended).
> 
> In my opinion, minimum font size feature is not whatever equivalent of text
> zoom, because unlike text zoom it hardly *limit* font size on all pages
> resulting in breaking initial proportions of text size in different text blocks
> on pages while these proportions is often method to accentuate different
> importance of text in different text blocks.
> 
I never noticed this but I can only speak for myself. I only filter out unreadably small letters.

Comment 8

[Marat Tanalin]

(In reply to comment #7)
> I noticed that in branch you need to scroll down to enlarge, but in Minefield
> it is the opposite (scroll up). I think this is a separate bug (if not
> intended).

It's the recently fixed bug 141476 *not* concerned with the current one.

Comment 9

[bugzilla.mozilla.org]

ccing myself and also requesting an option to switch back to FF2 behavior or an otpion to choose which zoom behavior is the default one for Menu, shortcut and mousewheel interaction

thanks

Comment 10

[Tim Babych]

there were lots of up to the point comments in bug 389628, and there is no need for me to retype them here. ccing myself and hoping that Fx drivers will not throw out such a great advantage over IE and Opera as nice Text Sizing is.

Comment 11

[Przemyslaw Bialik]

Adjusting description, raising severity, adding regression keyword (full page zoom is closer to being another feature than better text size zoom) and access keyword (text size zoom allowed to set text size way bigger than full page zoom does and so this bug affects users with vision disability) + asking for blocking‑firefox3.

Comment 12

[Mike Connor [:mconnor]]

access is a bogus keyword.  for users with visual disabilities adjusting the font size settings including minimum font size is a vastly better answer than zooming every page with small text.

this is not a regression in that it was an intended change.  an intended change to a feature does not qualify as a regression.

there's an about:config pref for the mousewheel behaviour, in any case.

also, the scroll direction is an intended change to sync with other apps behaviour, don't have the bug offhand.

Comment 13

[Dão Gottwald [:dao]]

(In reply to comment #12)
> access is a bogus keyword.  for users with visual disabilities adjusting the
> font size settings including minimum font size is a vastly better answer than
> zooming every page with small text.

Apparently the minimum font size feature is not very well known. Can we do something about that? Maybe adding it to "Preferences > Content > Fonts & Colors" instead of "Preferences > Content > Fonts & Colors > Advanced..."?

> also, the scroll direction is an intended change to sync with other apps
> behaviour, don't have the bug offhand.

bug 141476

Comment 14

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

As summarized, this bug doesn't block Firefox 3. However:

> Apparently the minimum font size feature is not very well known. Can we do
> something about that? Maybe adding it to "Preferences > Content > Fonts &
> Colors" instead of "Preferences > Content > Fonts & Colors > Advanced..."?

Yeah, there was discussion around Firefox 2 about making that dialog a lot smarter and easier to use, and I'd support some work there. Feels like a separate bug, though.

I'm sure that an extension will crop up that adds text-only zooming back into our UI, and I'd totally encourage it. From a UI perspective, though, combining options for text and full zoom just ends up being confusing for primary chrome, so we're going with the type of zoom that most people recognize as "zoom".

There was another thought a while back to have the first two zoom steps do text only, then move into full zoom. Dunno if there's a bug on that, but working that way might end up in a clever, best-of-both worlds compromise.

Comment 15

[Przemyslaw Bialik]

(In reply to comment #12)

> there's an about:config pref for the mousewheel behaviour, in any case.
ahh yeah (mousewheel.withcontrolkey.action:3) so similar pref could be added to switch zoom behaviour ;)
anyway this tip could be added to relnote

Comment 16

[Marat Tanalin]

(In reply to comment #14)
Mike, the problem is that people who needs to increase text want to increase *text only*. Full page zoom is useless for many people (assuming many Firefox users that are not Opera users due to zoom behavior exactly) and particulaly for mentioned people including your truly.

They do not want to look to distorted images (that are distorted in fact, particularly due to current nearest-neighbour interpolation instead of bicubic or at least bilinear one) in content area, they want just text readability while keeping images in *original pixel-to-pixel (logical to physical) quality*.

Full page zoom is just a technical feature (and it's good to have it in Firefox as a whole), but *not advantage* at all, and there are no reasons to make it default way to zoom.

So, the way to switch zooming behavior *should* exist -- and exist *out-of-the-box* (without any extensions) -- it's very important for such needed and useful thing. If such way will be hybrid zooming (text zoom at the few first steps and full page zoom at the next steps as mentioned in other comments) then the GUI preference to specify the *number* of these first steps should exist. In this case user will be able to set this number to a high value (20, for example) and actually switch full page zoom off in most cases.

It make sense to take into account that the real text zooming step dramatically depending on percents or ems CSS units is used to specify font size of HTML document, so probably zoom *ratio* (between two adjacent steps) should be a pref instead of the steps number. Thanks.

Comment 17

[Marat Tanalin]

"(between two adjacent steps)" in my latest comment is intended to read as "(between first and last text sizes in text zooming interval)".

Comment 18

[Jerome Dahdah]

Please don't change the default behavior of zoom that has now been a Firefox standard for years. The full page zoom is laggy, distorts the images in ugly ways (no anti-aliasing), makes me need to scroll horizontally and for some reason kicks up my CPU fan to 100%. And if you have to stick with it, then at least make it an option. The "mousewheel.withcontrolkey.action:3" option is not an option, it's a "hack" that advanced users can use. Someone here ask their mom to edit about:config and see how that goes.

Comment 19

[Mark Parnell]

FWIW I too want an easy way to set it back to use text zoom instead of page zoom. I have tried the mousewheel.withcontrolkey.action change and while it does work, it is far from an ideal solution. As Jerome said, it's really only an option for advanced users; in addition to that there is still no easy way to set the font sizes back to the default after zooming (i.e. Ctrl+0), and I would prefer to use the keyboard rather than the mouse.

I'm not even necessarily opposed to setting page zoom as the default, as long as there is an easy way to change it back for those of us who wish to do so.

Comment 20

[Sk]

A lot of Firefox users need the option to return *old* zoom behavior back. At least via about:config if it's can't be changed for marketing purposes.

That "mousewheel.withcontrolkey.action:3" key is some kind of crutch. I want to zoom text only via ctrl++ and ctrl+- and ctrl+0.

Comment 21

[J.M. Hardin]

I'm honestly finding the whole page zoom to be the worst new feature in Firefox that I've ever found in a new version. I'm tempted to stick with Firefox 2 to avoid what many users could find as this complete breakage of a fundamental Firefox feature. I know many users who will try Firefox 3 when it comes out, and if they lose text zooming in favor of page zooming (either by the keyboard and/or through an extension) they will go back to Firefox 2 and never upgrade.

I'm not sure why this was changed, I'm not sure what the decision making process was, but it needs to be made an option. Anything less will just **** users off.Especially since the mousewheel hack isn't reliable on all computers. It's certainly a crapshoot on my Linux box, which leaves me with either too small text or scrolling like mad just to read an article that I don't want to open to full screen.

Comment 22

[Derek]

I think the full page zoom also sucks, sometimes pictures on pages are already large enough to see but you need only to zoom the text on the page, so a toggle option is more then needed, I hate full page zoom in every browser that uses it to be honest, Most people would rather just zoom the text, now the few who can barely see like the impaired may need full page zoom, but for the majority of people all they need to zoom is the text on the page to make it more readable. That is why it needs to be an option.

Comment 23

[Peter van der Woude [:Peter6]]

Stop adding useless comments please.
It is clear enough that users wish to have the option to use textzoom-only and mozilla folks are aware of this.

Comment 24

[Loci]

Было-бы прекрасно если так. Текст-зум очень полезная штука, и в некоторых случаях подходить только он.

Comment 25

[Tim Babych]

Translation of the above into English: "It would be great. Text Zoom is uber-useful thing and can not be replaced in some cases".

Myself I will add that IMO people are flooding this bug with complaints because they do not see "assigned" or "blocking" or "having target-milestone" here and consider it therefore abandoned.

Comment 26

[Reed Loden [:reed]]

(In reply to comment #25)
> Myself I will add that IMO people are flooding this bug with complaints because
> they do not see "assigned" or "blocking" or "having target-milestone" here and
> consider it therefore abandoned.

Well, it *is* currently abandoned. Nobody has stepped forward to write a patch for it, and the bug was minused for blocking. SeaMonkey has done a good job of supporting both types of zoom in bug 405133. If somebody can write a patch for Firefox, maybe you can convince mconnor / beltzner.

Comment 27

[Emin]

Text zoom is a very important feature, not full page zoom. I suggest it should be optional and text zoom should be default.

Comment 28

[Alexey]

Having ability to resize only text is needed very much. And I support that this should be on by default - not full page zoom. Full page zoom is good for mobile devices - not desktop ones.

Comment 29

[Boris]

Text zoom is very important, I agree

Comment 30

[Danny]

It would be great to have text zoom back as a default feature. Full page zoom makes small pictures look ugly. It would be ideal if there is a switch in "Preferences".

Comment 31

[Juriy Ogijenko]

Add this option to preferences and more people will be "happy".

Comment 32

[Biju]

adding [parity-IE]
as IE7 has both "Page > Zoom" and "Page > Text Size"
and "Text Size" options are 
"Largest", "Large", "Medium", "Smaller", "Smallest"

I keeping default control key for full page zoom.
modified mousewheel.withaltkey.action = 3, to use altkey for text zooming.
So now I am lacking only ability to RESET text zoom.

Comment 33

[Ilya]

I agree that removing text zoom (which so many users already got used to) is a very bad idea. Enlarging the text only makes pages be much more readable than stretching pictures. So text zoom must be enabled by default, and a full-page zoom can be an extra feature, I think.

Comment 34

[klay]

Text zoom is very important, I agree

Comment 35

[Sevka]

Text zoom is very important

Comment 36

[Reed Loden [:reed]]

Put this back on the list for consideration. Personally, doing what IE does sounds reasonable.

Comment 37

[Dão Gottwald [:dao]]

IE's full page zoom is crap. There isn't really anything new that would justify a re-nomination.

Comment 38

[Marat Tanalin]

(In reply to comment #36)
> doing what IE does sounds reasonable.

Reed, it's very important to not add "just one more" menu, but make it possible to *full switch* between two zoom methods. Such switching should provide  ability to zoom via [CTRL]+[+/-/Wheel] with *chosen* method while not making one of two zoom methods hardly imposed in any way including all (or one of) CTRL-shortcuts.

IE7 provides only full page zoom via [CTRL]+[+/-/Wheel], and there is no way to use the same shortcuts to zoom text only; text resizing in IE7 is possible only via menu that is nearly useless and not enough for purposes requested in the context of this bug.

Fast switching of preferable zoom method directly from the "Zoom" menu (Zoom -> Zoom method -> Text only / Full page) would be best solution.

Comment 39

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

Adding an option(In reply to comment #38)
> Fast switching of preferable zoom method directly from the "Zoom" menu (Zoom ->
> Zoom method -> Text only / Full page) would be best solution.

This is the best option I've seen. The menuItem could be stateful, like:

    Zoom In
    Zoom Out
    --------------
    Reset
    Zoom text only

^ zooms full page

    Zoom In
    Zoom Out
    --------------
    Reset
  x Zoom text only

^ zooms text only

Comment 40

[Po-chiang Chao [:bobchao]]

(In reply to comment #39)
How about this, which keeps text-zoom as default?

  Zoom In
  Zoom Out
  --------------
  Reset
  Zoom full page

Comment 41

[Marat Tanalin]

(In reply to comment #39)
Yes, Mike, your variant is not bad. Though I personally would prefer more clear switching with two radio buttons and first one checked by default, for example:

    Zoom In
    Zoom Out
    --------------
    Reset
    --------------
    Zoom method | x Text only
                |   Full page

    this is submenu ^

Comment 42

[Biju]

A separator between "Reset" and "Zoom ..." item as in comment #41 is advisable, as it reduce accidentally clicking of zoom options when trying to click on "Reset".

Defaulting to "Full page" can reduce complaints of switchers from IE and Opera.
If we want to give significance to "Text only" zoom, name options as 
* "Advanced Text Only Zoom"
* "Simple Full page Zoom"

Comment 43

[J.M. Hardin]

I understand wanting to limit complaints from switchers, but what about all of us who have been using Firefox since 1.5 came out? We'll need to make it *very* clear how to get our beloved text zoom back as the normal behavior. If many users are like we we'll only go to page zoom kicking and screaming, and I'm still needing a true text zoom (with the keyboard, not the rodent) to be returned before I return to testing Fx3 too much.

Comment 45

[Brandon W Maister]

Some actual arguments for why text-only-zoom should be an option in addition to page zoom. One: google reader. I've got pretty lousy vision and I like the text REALLY big. gReader uses a pixel value for the sidebar (AFAICT) and that means that when you increase the text size, after a certain point the UI actually breaks--the scroll controls disappear. And for every website that uses pixels to measure sidebar widths, the content becomes very difficult to read after high magnification.

Relatedly, a site that uses pixels to define the width of the main content will require horizontal scrolling to successfully read the body.

These are both things that effectively create a much smaller maximum magnification than is possible with "just text" zoom, and is therefore bad for accessibility. Right?

Page zoom is fantastic for some things (e.g. nytimes.com) but less productive than text zoom for others (e.g. any article on itwire.com, whose articles are pixel defined and start off at nearly the full width of my monitor).

Having both page- and text-zoom would be fantastic, though. I can think of so many times where i would use both, but i don't think you need examples since i doubt anybody's going to *take out* page zoom :)

I do think that page zoom should be default, but i think that text zoom should be assigned a default key combo, and personally recommend the key-combo of <ctrl>+<shift>+* (currently used for history) or as a second choice just <shift>+* for text zoom.

<shift>, in addition to being right next to <ctrl>, already has the connotation of "bigger text" in english-speaking countries. I don't know about other places. 

I hope that this isn't just spam, I do believe that there are very good reasons for making this available, more than just "OH! Different! No!"

Comment 46

[Biju]

Also please see Bug 407619, Bug 250415 for thunderbird...

Comment 47

[Eugen]

I think it would be better if you include both zooming options because there are many people using Firefox, and not all of them will like this feauture.

Comment 48

[Rohan]

Any progress? I won't see final version with stupid new zoom! 

Comment 49

[Rohan]

Any progress? I won't see final version with stupid new zoom! 

Comment 50

[J.M. Hardin]

Comment 23 says Mozilla folks are aware of the issue, but this isn't even being included in the annoying bug list in the nightly build thread. The overall silence is not heartening. I guess we'll have to make a point in taking part in the next Bug Day and specifically testing the Zoom feature. Yes, it will fail, and it may seem rhetorical, but I'm seeing little other mechanism to get any response on an issue which is clearly quite important to many users.

I'm sorry, but severity Normal? My ****. Can we at least get the severity changed on this bug to something approaching how so many users view it?

Comment 51

[Peter van der Woude [:Peter6]]

Yeah, I could change the severity to enhancement, because that is what is is.
But that won't make a difference though.
You have to see this is perspective, currently it's all hands on memory leaks, defragmentation and performance.
If Mozilla runs out out spare hands this won't make it into 3.0.
So either do it yourself or find a volunteer to do it, or find someone to write an extension to implement what you wish.

Comment 52

[Alexander Derazhne]

As solution I forbid my FF to automatically update. I'll sitting on V2 because V3 is unusable for me :-(.
BTW: IE (for example) allow to put anything at the toolbar. But I don't find ability to put text-size controls to toolbar in the FF :-(. It's not too convenient to do it trough menu or switch hands from mouse to keyboard...
 

Comment 53

[Bill Gianopoulos [:WG9s]]

(In reply to comment #39)
> This is the best option I've seen. The menuItem could be stateful, like:
> 
>     Zoom In
>     Zoom Out
>     --------------
>     Reset
>     Zoom text only
> 
> ^ zooms full page
> 
>     Zoom In
>     Zoom Out
>     --------------
>     Reset
>   x Zoom text only
> 
> ^ zooms text only
> 
I like this idea.  However, I think it would be better to have it as a preference as well, so that the selected zoom mode persists across browser restarts.   This menu choice would then toggle the preference.

Comment 54

[Radek 'sysKin' Czyz]

Can I anti-vote please?

I've been using full zoom since it became possible and I never ever missed old weird behaviour. Image zoom might be ugly, but I manage to switch it back to 100% if I ever need to appreciate a high quality image, if I need to.

Anyway, changing font size while keeping anything but font size unchanged is conceptually as weird as doing the opposite. Either you want the page bigger or you don't.

Bloating options or menus for that is pointless, most users don't even open the options dialogue in their lives (and when they do, they want to change an option they expect there to be beforehand). People who actually understand the difference between font size and everything-else-size are people capable of installing extensions for whatever zoom-one-thing-but-not-other they fancy.

Apologies for another advocacy post, just wanted to make it clear that there are people with opposite views in that matter.

Comment 55

[Timur Timirkhanov]

Attachment: Proof of concept patch

If you like it, I'll go on working, but I will need some help.

Comment 56

[Rohan]

How it's work? It work like classical zoom from FF2.0?
How we can test it and does it will be include in next beta?

Comment 57

[Timur Timirkhanov]

>How it's work?
It's a patch, so you have to compile Firefox trunk with the patch applied.
>It work like classical zoom from FF2.0?
No, it adds a preference in about:config. Depending on the preference's value it zooms either full page or text only (like in FF 2.0).
>How we can test it
See answer to your first question, please.
>does it will be include in next beta?
Have no idea about this, sorry. But I'd love to have it.

Comment 58

[Tim Babych]

You can grab precompiled beta2 and just patch viewZoomOverlay.js in toolkit.jar
The new preference name is "fullZoom.use"

Thank you, Timur, for your effort!

Comment 59

[Rohan]

Which mode use by default?
Timur, please if you have ready to use build FF, give us link.

Comment 60

[Timur Timirkhanov]

>Which mode use by default?
Either you'd like to.
>Timur, please if you have ready to use build FF, give us link.
I don't have any. You should follow Tim Babych's advice if you're willing to test it. Any feedback is welcome.

Comment 61

[J.M. Hardin]

Can we get a clarification on what's involved in "patch viewZoomOverlay.js in toolkit.jar". While I've compiled code for other programs I've never applied patches and I'm not sure what I have to do to be able to test this change.

Thank you, Timur, for your work in resolving this "bug". We probably owe you several beers for this.

Comment 62

[Dão Gottwald [:dao]]

Untested tryserver builds with the patch applied:
https://build.mozilla.org/tryserver-builds/2007-12-22_07:[e-mail]-1198339045/

Comment 63

[Timur Timirkhanov]

To test feature, you grab the builds from the link in Dao's post, go to about:config and create a new boolean preference "fullZoom.use". If set to true, you'll have full zoom on mousewheel scroll and keypress. if set to false, you'll get text-only zoom on keypres only. I'm thinking over how to realize it for mousewheel.

Comment 64

[philippe (part-time)]

Timur,
That is HUGE progress.

Comment 65

[Sk]

Timur, thanks for your efforts!

What for me, I would be happy to have text-zoom on -,+ keys, and full-zoom on mousewheel. The only thing - I would like to reset all the zooms by pressing ctrl+0.

Comment 66

[Timur Timirkhanov]

>What for me, I would be happy to have text-zoom on -,+ keys, and full-zoom on >mousewheel. The only thing - I would like to reset all the zooms by pressing
>ctrl+0.

This is what my patch does. Unless you change pref mousewheel.withcontrolkey.action to 3 from 5 (default), you're able to do what you've described.

Comment 67

[Elmar Ludwig]

Attachment: complete patch v1

This is a complete version of Timur's proof of concept patch. It adds a menu item as proposed by Mike Beltzner in comment #39, renames the pref from "fullZoom.use" to "toolkit.zoomManager.textOnly" and switches Ctrl+Mousewheel to always use the currently selected zoom method (text only or full zoom). If the last part is not wanted, simply drop the changes to nsEventStateManager.cpp.

Warning: I don't have a lot of time over Christmas, so this is not really tested. But I'm asking for review anyway to get some feedback...

Comment 68

[Elmar Ludwig]

Note: My patch makes it so that the current zoom factor is switched over to the other zoom style whenever the menu item is toggled, i.e. if you have used full zoom to resize the page and switch to "text only zoom", full zomm is reset and the zoom factor is applied to text zoom instead (and vice versa). This seemed most logical to me.

Comment 69

[Bill Gianopoulos [:WG9s]]

(In reply to comment #68)
> Note: My patch makes it so that the current zoom factor is switched over to the
> other zoom style whenever the menu item is toggled, i.e. if you have used full
> zoom to resize the page and switch to "text only zoom", full zomm is reset and
> the zoom factor is applied to text zoom instead (and vice versa). This seemed
> most logical to me.
> 

That makes the most sense to me also.  I have done quite a bit of testing on this, and so far no issues.

Comment 70

[Philip Chee]

(In reply to comment #67)

> renames the pref from "fullZoom.use" to "toolkit.zoomManager.textOnly"

The SeaMonkey version of this bug uses "browser.zoom.full" instead. See: <https://bugzilla.mozilla.org/attachment.cgi?id=290515>

I suggest that you use the same pref to avoid confusion.

Comment 71

[Boris Zbarsky [:bzbarsky]]

Wouldn't it make more sense to send the right event to start with instead of hacking one of the event-handling codepaths?

Comment 72

[Elmar Ludwig]

(In reply to comment #70)
> The SeaMonkey version of this bug uses "browser.zoom.full" instead.
> I suggest that you use the same pref to avoid confusion.

Probably a good idea, I hadn't noticed that.

(In reply to comment #71)
> Wouldn't it make more sense to send the right event to start with instead of
> hacking one of the event-handling codepaths?

I played around with that a bit, but the problem is that the user can assign the MOUSE_SCROLL_TEXTSIZE and MOUSE_SCROLL_FULLZOOM events to any mousewheel action, not just mousewheel.withcontrolkey. So we would have to enumerate these in the ZoomManager and change them accordingly when the zoom type changes, which is kind of ugly IMHO.

The other approach would be to eliminate the distinction between MOUSE_SCROLL_TEXTSIZE and MOUSE_SCROLL_FULLZOOM that was introduced in the fix for bug 401214 altogether and have only one MOUSE_SCROLL_ZOOM action that does one or the other depending on the pref setting. This is actually similar to what the current patch does.

If you prefer the first approach, I can update the patch accordingly.

Comment 73

[Boris Zbarsky [:bzbarsky]]

Assuming the pref setting defaults to text zoom in apps that are not using the new zoom manager UI, having the same pref control the behavior of mouse zoom and menu zoom makes a lot of sense to me.

Comment 74

[Timur Timirkhanov]

I'd like to see it in the following way:

+  toggleZoom: function ZoomManager_toggleZoom() {
+    var zoomValue = this.fullZoom;
+
+    this.fullZoom = 1;
+    if (this.textOnly) {
+    //set mousewheel.withcontrolkey.action to 3
+    }
+    else {
+    //set mousewheel.withcontrolkey.action to 5
+    }
+    this.textOnly = !this.textOnly;
+    this.fullZoom = zoomValue;
+  },

I think this will give more flexibility to more advanced users who might want to have both full-zoom on mouse wheel and text-zoom on keyboard (or vice versa) at the same time.

Comment 75

[Rohan]

"toolkit.zoomManager.textOnly", false"
By default should use text zoom!! Please fix it!

Comment 76

[Boris Zbarsky [:bzbarsky]]

Rohan, please see <https://bugzilla.mozilla.org/etiquette.html>.

Timur, that's he approach I initially suggested, but it has the problem Elmar pointed out: zoom need not be bound to mousewheel.withcontrolkey.action.

If we really think there are people who sometimes want text zoom and sometimes full zoom (even though the two interact with each other badly) then the right solution seems to be to have one "zoom" event in the ESM, with a single pref that controls what sort of zoom that event does, and a separate pref for what the zoom manager does.  Or something.  That seems like too much complexity to me, frankly.

Comment 77

[Jerome Dahdah]

First off, thank you Timur and Elmar for getting this thing off its feet, this is really great.

I would have all keys do the same thing, so as to keep the functionality clear. I see no problem in using two different prefs for ctrl++ and ctrl+mousewheel (as in two seperate about:config entries), but the GUI option ("zoom text only") should switch both prefs accordingly to coincide. Advanced users can then have the freedom to make ctrl++ and ctrl+mousewheel behave differently, while regular users have one understandable toggle, independent of their preference of interaction.

I also agree with Elmar's suggestion in comment #68.

Comment 78

[Tuukka Tolvanen (sp3000)]

(In reply to beltzner's comment #14)
> There was another thought a while back to have the first two zoom steps do text
> only, then move into full zoom. Dunno if there's a bug on that, but working

There's a patch in bug 405310 for that. I feel it pretty much always does what I want, but the downside is that it could appear a bit surprising.

Comment 79

[Elmar Ludwig]

Attachment: complete patch v1.1

Updated patch (v1.1). This patch uses the pref name "browser.zoom.full" to be consistent with attachment #290515 [details] [diff] [review] of bug 405133 (false by default and set to true for Firefox), replaces MOUSE_SCROLL_TEXTSIZE and MOUSE_SCROLL_FULLZOOM with MOUSE_SCROLL_ZOOM in the EventStateManager and reverts the default value of "mousewheel.withcontrolkey.action" to 3 for Firefox. The rest of the patch is unchanged, except for the changed property of the ZoomManager ("useFullZoom").

Comment 80

[Elmar Ludwig]

(In reply to comment #74)
> I think this will give more flexibility to more advanced users who might want
> to have both full-zoom on mouse wheel and text-zoom on keyboard (or vice versa)
> at the same time.

I tend to agree with Boris here that allowing this adds too much complexity for too little value, especially given the fact that currently only one zoom setting is remembered per site.

Comment 81

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 294579 [details] [diff] [review]
complete patch v1.1

Looks reasonable.  Thanks!

Comment 82

[Biju]

First of all thanks for the FIX.

But I still wish advance user have capability of controlling both zoom.
Say alt+mousewheel for textzoom
and ctrl+mousewheel for fullzoom

I dont think that code will be complex, 
I am suggesting the following way.
Yes there is a problem of saving the zoom value
if that is important, then code for 'toggleZoom' can be kept 
as in patch attachment 294579 [details] [diff] [review]

====== proposed code ====

pref("mousewheel.withcontrolkey.action",5);


*******************
 enum {
  MOUSE_SCROLL_N_LINES,
  MOUSE_SCROLL_PAGE,
  MOUSE_SCROLL_HISTORY,
  MOUSE_SCROLL_TEXTSIZE,
  MOUSE_SCROLL_PIXELS,
  MOUSE_SCROLL_FULLZOOM
 };


************************

         break;
 
       case MOUSE_SCROLL_HISTORY:
         {
           DoScrollHistory(msEvent->delta);
         }
         break;
 
       case MOUSE_SCROLL_TEXTSIZE:
         {
           DoScrollZoom(aTargetFrame, msEvent->delta, 0);
         }
         break;
 
       case MOUSE_SCROLL_FULLZOOM:
         {
           DoScrollZoom(aTargetFrame, msEvent->delta, 1);
         }
         break;
 
       default:  // Including -1 (do nothing)
         break;
       }
       *aStatus = nsEventStatus_eConsumeNoDefault;
 


******************************************


 void
 nsEventStateManager::DoScrollZoom(nsIFrame *aTargetFrame,
                                   PRInt32 adjustment,
                                   PRInt32 isFullZoom)
 {
   // Exclude form controls and XUL content.
   nsIContent *content = aTargetFrame->GetContent();
   if (content &&
       !content->IsNodeOfType(nsINode::eHTML_FORM_CONTROL) &&
       !content->IsNodeOfType(nsINode::eXUL))
     {
       // positive adjustment to decrease zoom, negative to increase
       PRInt32 change = (adjustment > 0) ? -1 : 1;
 
       if (isFullZoom)
         ChangeFullZoom(change);
       else
         ChangeTextSize(change);
     }
 }


**********************************
   reset: function ZoomManager_reset() {
      getBrowser().markupDocumentViewer.textZoom = 1;
      getBrowser().markupDocumentViewer.fullZoom = 1;
   },

   toggleZoom: function ZoomManager_toggleZoom() {
     this.useFullZoom = !this.useFullZoom;
   },

Comment 83

[Biju]

oops missed...
also we need 

pref("mousewheel.withaltkey.action",3);

Comment 84

[Szabolcs Horvát]

How will this patch interact with extensions that provide alternative user interfaces to zooming, such as Mouse Gestures?  Will it switch the zoom type done by the extension too?  (It would be more consistent if it worked that way.)

Comment 85

[Elmar Ludwig]

(In reply to comment #82)
> I dont think that code will be complex, I am suggesting the following way.

Your suggestion has a number of problems that can really confuse users:

1. Toggling the menu item does not change what Ctrl+Mousewheel does. Normal users will likely not understand why the keyboard shortcuts and the mouse wheel now behave differently.

2. Toggling the menu and switching tabs back and forth will change the zoom in unexpected ways. E.g. if you have full zoom selected and use this to zoom in one step and then switch to text zoom will cause the tab to be displayed with both full zoom and text zoom applied the next time you switch to the tab, while other tabs of the same URL will show a different zoom level.

This is why I like the behaviour described in comment #68. It is simple to understand and avoids side effects of this kind.

(In reply to comment #84)
> How will this patch interact with extensions that provide alternative user
> interfaces to zooming, such as Mouse Gestures?  Will it switch the zoom type
> done by the extension too?  (It would be more consistent if it worked that
> way.)

That depends on how the extension works. If it is using the ZoomManager, it should automatically use the currently selected zoom type. But if it directly sets either text zoom or full zoom, it will continue to work but be unaffected by changing the zoom type.

Comment 86

[Elmar Ludwig]

Attachment: complete patch v1.2

Sorry, the last patch was missing a simple change to nsEventStateManager.h to bring it in line with the changed method name in nsEventStateManager.cpp:

> -  void DoScrollTextsize(nsIFrame *aTargetFrame, PRInt32 adjustment);
> -  void DoScrollFullZoom(nsIFrame *aTargetFrame, PRInt32 adjustment);
> +  void DoScrollZoom(nsIFrame *aTargetFrame, PRInt32 adjustment);

This was just omitted from the diff by accident and is now included.

carrying over sr=bzbarsky

Comment 87

[Peter Lairo]

I've read almost all comments and am not sure if the question of which default has been settled.

Reasons to make text zoom the default:

1. Why do we zoom? IMO, almost always (at a minimum >50%) to be able to better read text that is too small.

2. Keep layout? HTML is designed to accommodate all kinds of resolutions, monitor sizes, dpi settings, font-size settings, etc. Layout is for paper output. Web pages must "go with the flow" (in accordance with our font size preferences).

3. Do we need to see low-res images zoomed into a pixellated mess? Almost never.

4. Full page zoom causes the dreaded horizontal scrolling. Very little could justify that high price.

Conclusion: Text zoom is more useful and should be the default.

People switching from IE can either change the setting to their habitual (but inferior) behavior, or rejoice in the more useful default setting (text zoom).

Comment 88

[Biju]

(In reply to comment #85)
> (In reply to comment #82)
> > I dont think that code will be complex, I am suggesting the following way.
> 
> Your suggestion has a number of problems that can really confuse users:
> 
> 1. Toggling the menu item does not change what Ctrl+Mousewheel does. N

I think I have solution for that also.
Please following logic with ZOOM and ALTZOOM

====== new proposed code ====

pref("mousewheel.withcontrolkey.action",3);
pref("mousewheel.withaltkey.action",5);


*******************
 enum {
  MOUSE_SCROLL_N_LINES,
  MOUSE_SCROLL_PAGE,
  MOUSE_SCROLL_HISTORY,
  MOUSE_SCROLL_ZOOM,
  MOUSE_SCROLL_PIXELS,
  MOUSE_SCROLL_ALTZOOM
 };


************************

         break;

       case MOUSE_SCROLL_HISTORY:
         {
           DoScrollHistory(msEvent->delta);
         }
         break;

       case MOUSE_SCROLL_ZOOM:
       case MOUSE_SCROLL_ALTZOOM:
         {
           DoScrollZoom(aTargetFrame, msEvent->delta, action);
         }
         break;

       default:  // Including -1 (do nothing)
         break;
       }
       *aStatus = nsEventStatus_eConsumeNoDefault;



******************************************


 void
 nsEventStateManager::DoScrollZoom(nsIFrame *aTargetFrame,
                                   PRInt32 adjustment,
                                   PRInt32 action)
 {
   // Exclude form controls and XUL content.
   nsIContent *content = aTargetFrame->GetContent();
   if (content &&
       !content->IsNodeOfType(nsINode::eHTML_FORM_CONTROL) &&
       !content->IsNodeOfType(nsINode::eXUL))
     {
       // positive adjustment to decrease zoom, negative to increase
       PRInt32 change = (adjustment > 0) ? -1 : 1;


       if (nsContentUtils::GetBoolPref("browser.zoom.full")){
          if (action == MOUSE_SCROLL_ZOOM) ChangeFullZoom(change);
          else ChangeTextSize(change);
      }else{
          if (action == MOUSE_SCROLL_ZOOM) ChangeTextSize(change);
          else ChangeFullZoom(change);
     }
 }

Comment 89

[Marat Tanalin]

Just want to say that if to make possible both zoom methods with different modifier keys, then SHIFT+Mousewheel should used instead of ALT+Mousewheel due to pressing ALT results in activation of main application menu in Windows that may be unusable.

Comment 90

[Elmar Ludwig]

(In reply to comment #87)
> I've read almost all comments and am not sure if the question of which default
> has been settled.

This was decided in bug 389628 comment #64 AFAIK and unless that decision is reversed, full zoom will remain the default.

(In reply to comment #88)
> I think I have solution for that also.

Although something along those lines might work, it still doesn't solve the problem of persisting only one zoom level. If you really feel that you need the ability to have both zoom types on different mouse wheel actions, please file a separate enhancement request for this and let's focus on the issue mentioned in comment #0 for this bug.

Comment 91

[Ryan VanderMeulen [:RyanVM]]

v1.2 works quite nicely. I'm not seeing any obvious bugs with it.

One nit, I tend to agree with comment #42 that there should be a spacer between the reset and zoom text only options to reduce the chances of accidentally clicking the wrong thing. Plus, they're really not related to each other, so they shouldn't be grouped together.

Comment 92

[Elmar Ludwig]

Mike Beltzner: Do you have time to ui-review this?

The patch here basically implements your suggestion from comment #39, but there is one issue that needs to be decided before this goes forward: What should happen when the user switches the zoom type on an already zoomed page?

As already explained in comment #68, my patch makes it so that the current zoom factor is switched over to the other zoom style in this case, i.e. if you have used full zoom to resize the page and switch to "text only zoom", full zoom is reset and the zoom factor is applied to text zoom instead (and vice versa).

(In reply to comment #91)
> One nit, I tend to agree with comment #42 that there should be a spacer
> between the reset and zoom text only options to reduce the chances of
> accidentally clicking the wrong thing.

I thought about this as well. This is trivial to add, if Mike agrees that this should be done.

Comment 93

[Bill Gianopoulos [:WG9s]]

Linux and Windows test builds that include this patch are available at:

http://www.wg9s.com/mozilla/firefox/

Comment 94

[Sk]

So far, so good. Acts like it should be, in my opinion.

It's probably not for this bug, but in Fx2 ctrl+mousescroll_up decreases zoom, in this build it increases zoom. Probably it's not good for old users.

Comment 95

[Philip Chee]

> It's probably not for this bug, but in Fx2 ctrl+mousescroll_up decreases zoom,
> in this build it increases zoom. Probably it's not good for old users.

This change in mouse direction is deliberate. Nobody else does it the Firefox 2 way. Even MSIE7 has fallen in line with everyone else.

Comment 96

[Philip Hofstetter]

As long as an extension could restore the old text zoom feature (or provide additional shortcuts for that), I'm fine with the way it's currently handled. But text-zoom for me is a really important feature. Whenever I'm reading longer texts online, I'm increasing the font-size. This actually allows me to concentrate much better on the meaning of the text itself because i'm not constantly wasting energy in trying to follow the words.

This also is based in the fact that many websites have lines that are way too wide to be read comfortably. 

Now with Page-Zoom, the text gets larger, but so does the witdh of the lines, thus defeating the purpose of why I have increased the font-size in the first place.

As it stands now, whenever I'm about to read a longer text online, I'm opening Safari, pasting to URL there and using the text-zoom feature to actually be able to read the article.

If this bug does not reach a consensus in how to re-implement text-zoom UI-wise, at least provide Extensions with a means to bring it back.

Philip

Comment 97

[Alexander Derazhne]

Is it possible to put textZoom controls on the toolbar? It will give ability to fast control zoom with mouse only. Current implementation require some levels of menu (not convenient for persons who need zoom, is it?) or jumping hands to keyboard (my compact desktop model need both hands, probably, some notebook users have the some problems). 

Comment 98

[Bill Gianopoulos [:WG9s]]

That is what the QuickZoom extension is for:

https://addons.mozilla.org/en-US/firefox/addon/2540

Comment 99

[Alexander Derazhne]

Excellent! Thank You a lot, Bill!
But next: is it possible to set some special mode for zoom, with re-place graphics and other elements? Some sites designed with somewhere hard defined coordinates of elements and they are overlapped during zoom (atmel.com or inosmi.ru , for example).

Comment 100

[Sk]

So, according to (http://blog.mozilla.com/faaborg/2008/01/17/keyhole-coming-to-os-x-and-windows-themes/) there will be beta3 code freeze next week.

What prevents from this patch being installed in beta3? Everything ready for over 2 weeks now..

Comment 101

[Marat Tanalin]

Well, maybe some more (at first glance) important bugs exists...

So, dear Firefox developers, in some cases abstract importance is really not so important as *ratio* of importance and implementation *complexity*.

In case of this bug, as I see (though I may be mistaken), implementation is not a problem since there is a complete patch while there is a huge importance of requested functionality for many faithful Firefox users. And text zoom switcher is really good thing to implement in upcoming Beta 3. Thanks in advance.

Comment 102

[Elmar Ludwig]

Attachment: [checked in] patch v1.3 (l10n part)

Split out string changes, trying to land these before the l10n freeze.

Comment 103

[Elmar Ludwig]

Attachment: patch v1.3 (code + ui)

this is the complete patch v1.2 updated to trunk with two changes:

- split out l10n changes into a separate patch (not included here)
- menu separator between "Reset" and "Zoom Text Only" (as was suggested before)

carrying over sr=bzbarsky

Comment 104

[Reed Loden [:reed]]

Comment on attachment 298815 [details] [diff] [review]
[checked in] patch v1.3 (l10n part)

String changes to land before l10n freeze.

Comment 105

[Reed Loden [:reed]]

Checking in browser/locales/en-US/chrome/browser/browser.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.dtd,v  <--  browser.dtd
new revision: 1.90; previous revision: 1.89
done

Comment 106

[Ryan VanderMeulen [:RyanVM]]

Attachment: patch v1.3a (code + ui)

Recent changes to firefox.js caused this patch to bitrot. Here's an updated one with no functional changes to v1.3. Carrying over previous r? and ui-r+.

Comment 108

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

Comment on attachment 299415 [details] [diff] [review]
patch v1.3a (code + ui)

>Index: content/events/src/nsEventStateManager.cpp
> enum {
>  MOUSE_SCROLL_N_LINES,
>  MOUSE_SCROLL_PAGE,
>  MOUSE_SCROLL_HISTORY,
>- MOUSE_SCROLL_TEXTSIZE,
>- MOUSE_SCROLL_PIXELS,
>- MOUSE_SCROLL_FULLZOOM
>+ MOUSE_SCROLL_ZOOM,
>+ MOUSE_SCROLL_PIXELS
> };

We haven't shipped with fullZoom yet, so I guess this won't affect compatibility too much, but users that have set their zoom prefs to "5" will be broken here. I don't see any other in-tree apps that use 5 by default, but some may expose it in their pref UI (suite/common/pref/pref-mousewheel.xul does, it seems). Need to make sure that the relevant app owners are CCed, or that bugs are filed.

>Index: toolkit/content/viewZoomOverlay.js

>   get fullZoom ZoomManager_get_fullZoom() {
>-    return getBrowser().markupDocumentViewer.fullZoom;
>+    if (this.useFullZoom)
>+      return getBrowser().markupDocumentViewer.fullZoom;
>+    else
>+      return getBrowser().markupDocumentViewer.textZoom;

Remove the else-after-return. It seems confusing to have this property named "fullZoom" when it can now be either "text zoom" or "full zoom". I would rename it just "zoom" or "zoomLevel".

>   set fullZoom ZoomManager_set_fullZoom(aVal) {

>+    if (this.useFullZoom) {
>+      getBrowser().markupDocumentViewer.textZoom = 1;
>+      getBrowser().markupDocumentViewer.fullZoom = aVal;
>+    } else {
>+      getBrowser().markupDocumentViewer.textZoom = aVal;
>+      getBrowser().markupDocumentViewer.fullZoom = 1;
>+    }

nit: keep a reference to getBrowser().markupDocumentViewer in a local variable to avoid getting it twice?

>+  toggleZoom: function ZoomManager_toggleZoom() {
>+    var zoomValue = this.fullZoom;
>+
>+    this.fullZoom = 1;
>+    this.useFullZoom = !this.useFullZoom;
>+    this.fullZoom = zoomValue;
>+  },

What's the |this.fullZoom = 1;| for? The fullZoom setter already sets the other value to 1, so it shouldn't be necessary, right?

>+  updateMenu: function ZoomManager_updateMenu() {
>+    var menuItem = document.getElementById("toggle_zoom");
>+
>+    menuItem.setAttribute("checked", !this.useFullZoom);
>+  },

Can't have this toolkit file depend on the ID of a browser element... Since this doesn't really need to be part of the ZoomManager, I'd just move it to a browser file (browser-textZoom.js, in FullZoom looks like a suitable candidate).

The rest looks fine. I will r+ a patch with these comments addressed.

Comment 109

[Elmar Ludwig]

> We haven't shipped with fullZoom yet, so I guess this won't affect
> compatibility too much, but users that have set their zoom prefs to "5"
> will be broken here. I don't see any other in-tree apps that use 5 by
> default, but some may expose it in their pref UI.

Users that have changed the pref in about:config will know how to change it back, I think. But I didn't know that SeaMonkey has UI for this now. Alas, keeping both values makes the code kind of ugly... I had this in the first version of the patch that I discussed with bz in comment #71 ff. I personally think that having two zoom events was a mistake to begin with, so I would appreciate some feedback on this from the app owners.

> Remove the else-after-return

Will do.

> It seems confusing to have this property named "fullZoom" when it can
> now be either "text zoom" or "full zoom". I would rename it just "zoom"
> or "zoomLevel".

I thought about that already, but feared doing so would break extensions that access this property. If this is not a concern, I will change that.

> nit: keep a reference to getBrowser().markupDocumentViewer in a local
> variable to avoid getting it twice?

Will do.

> What's the |this.fullZoom = 1;| for? The fullZoom setter already sets the
> other value to 1, so it shouldn't be necessary, right?

Correct, this can be removed.

> Can't have this toolkit file depend on the ID of a browser element...Since
> this doesn't really need to be part of the ZoomManager, I'd just move it
> to a browser file (browser-textZoom.js, in FullZoom looks like a suitable
> candidate).

Yes, good catch. I will move this to browser-textZoom.js.

> The rest looks fine. I will r+ a patch with these comments addressed.

Thanks for your comments, I will post an updated patch tomorrow.

Comment 110

[Rohan]

Why mouse wheel back work as zoom out?  In FF2.0 mouse wheel back work as zoom in, and it was more logical, than now.

Comment 111

[Steve England [:stevee]]

(In reply to comment #110)
> Why mouse wheel back work as zoom out?  In FF2.0 mouse wheel back work as zoom
> in, and it was more logical, than now.

Parity with what other apps do iirc.

Comment 112

[Rohan]

parity with which application? Photoshop, for example use standard zoom --> back wheel = zoom in

Old zoom is more logical than new. "To" mean "Closer" in all word!
Look at Russian voting http://forum.mozilla-russia.org/viewtopic.php?pid=219202#p219202
70% want old zoom be default! 

Comment 113

[Marat Tanalin]

(In reply to comment #112)
Please don't spam this page. This bug is about another thing. Thanks.

Comment 114

[Elmar Ludwig]

Attachment: patch v1.4 (code + ui)

updated patch to address the points raised in comment #108:

- rename the "fullZoom" property to "zoom"
- move updateMenu() from toolkit to browser
- some minor changes as requested by Gavin

This patch has sr=bzbarsky and ui-r=beltzner. Do I need a separate review for the content/ part?

Comment 115

[Reed Loden [:reed]]

(In reply to comment #114)
> This patch has sr=bzbarsky and ui-r=beltzner. Do I need a separate review for
> the content/ part?

No, that's what bz's sr= was for (as far as I can tell), as browser and toolkit do not require superreview.

Comment 116

[Elmar Ludwig]

(In reply to comment #115)
> No, that's what bz's sr= was for (as far as I can tell), as browser and toolkit
> do not require superreview.

Fine.

If Gavin gives this patch r+ and there are no other objections, I will file a bug on SeaMonkey for the MOUSE_SCROLL_TEXTSIZE/MOUSE_SCROLL_FULLZOOM change.

Comment 117

[Dão Gottwald [:dao]]

Comment on attachment 302235 [details] [diff] [review]
patch v1.4 (code + ui)

>Index: browser/app/profile/firefox.js
> pref("fullZoom.minPercent", 50);
> pref("fullZoom.maxPercent", 300);
> pref("toolkit.zoomManager.fullZoomValues", ".5,.75,1,1.25,1.5,2,3");

These prefs have been introduced for full zoom specifically. You should not use them for text zoom or, if that's really what you want, rename them.

Comment 118

[Elmar Ludwig]

Is there any other reason why they should not be used for text zoom, apart from having beeing introduced for full zoom (which I find not a strong argument)? Text zoom used a different, fixed list of zoom steps (and mix/max values) in the past. These got removed by the patch in bug 389628 and were replaced with the prefs when the UI for full zoom was implemented.

Given that having a different list of zoom steps here would cause the zoom level to change (albeit slightly) when switching between zoom types, I'd rather not introduce a separate list for text zoom here. So, renaming the prefs seems like the best solution to me, if that is acceptable so late in the beta cycle.

Comment 119

[Roman R.]

Is there a way to simply specify a step instead of a list of zoom values (for both full page and text zoom)? I don't like the default step and it would be much easier for me and people who share my perception of the current settings to specify one value (the step) instead of modifying the list of 20 thousand numbers.

Comment 120

[Elmar Ludwig]

Attachment: patch v1.5 (code + ui)

same as v1.4 but with renamed fullZoom prefs as suggested by Dão

Comment 121

[Elmar Ludwig]

(In reply to comment #119)
> Is there a way to simply specify a step instead of a list of zoom values (for
> both full page and text zoom)?

No, there isn't (and that's not the topic of this bug). You can read bug 389628 for some of the discussion that lead to the current list.

Comment 122

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

Comment on attachment 302657 [details] [diff] [review]
patch v1.5 (code + ui)

Don't forget to file the SeaMonkey bug.

Comment 123

[Elmar Ludwig]

Filed bug 417141 for the changes that need to be done to the mouse wheel pref pane in SeaMonkey once this patch lands.

Comment 124

[Reed Loden [:reed]]

Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.277; previous revision: 1.276
done
Checking in browser/base/content/browser-menubar.inc;
/cvsroot/mozilla/browser/base/content/browser-menubar.inc,v  <--  browser-menubar.inc
new revision: 1.129; previous revision: 1.128
done
Checking in browser/base/content/browser-sets.inc;
/cvsroot/mozilla/browser/base/content/browser-sets.inc,v  <--  browser-sets.inc
new revision: 1.111; previous revision: 1.110
done
Checking in browser/base/content/browser-textZoom.js;
/cvsroot/mozilla/browser/base/content/browser-textZoom.js,v  <--  browser-textZoom.js
new revision: 1.8; previous revision: 1.7
done
Checking in content/events/src/nsEventStateManager.cpp;
/cvsroot/mozilla/content/events/src/nsEventStateManager.cpp,v  <--  nsEventStateManager.cpp
new revision: 1.729; previous revision: 1.728
done
Checking in content/events/src/nsEventStateManager.h;
/cvsroot/mozilla/content/events/src/nsEventStateManager.h,v  <--  nsEventStateManager.h
new revision: 1.162; previous revision: 1.161
done
Checking in modules/libpref/src/init/all.js;
/cvsroot/mozilla/modules/libpref/src/init/all.js,v  <--  all.js
new revision: 3.729; previous revision: 3.728
done
Checking in toolkit/content/viewZoomOverlay.js;
/cvsroot/mozilla/toolkit/content/viewZoomOverlay.js,v  <--  viewZoomOverlay.js
new revision: 1.10; previous revision: 1.9
done

Comment 125

[Gabriel Chadwick]

Shouldn't this be marked late l10n ?

Comment 126

[Ryan VanderMeulen [:RyanVM]]

The string changes went in at the end of January prior to the string freeze, no?

Comment 127

[Dão Gottwald [:dao]]

Similar to zoom.minPercent, browser.zoom.full should have been called zoom.full, since content reads it.

Comment 128

[Elmar Ludwig]

(In reply to comment #127)
> Similar to zoom.minPercent, browser.zoom.full should have been called
> zoom.full, since content reads it.

The patch is just using a SeaMonkey pref that already existed. If you think it is worth changing, please file a followup bug for this.

Comment 129

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022005 Minefield/3.0b4pre ID:2008022005

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008022004 Minefield/3.0b4pre ID:2008022004

Comment 131

[big]

Windows 98 allows me to use the text view. Now, Vista will not. I say find out how to link all browsers and all url sites including along with the headers to keep the same text size with one key or click? Tie it together otherwise it looks like Internet Explorer will be getting mine and others attentions boys, FIX The text tool back like it was so we can view it the normal way as it should be and include the zoom if you want but don;t make it the priority.

Comment 132

[Roman R.]

"text view", "link all browsers and all url sites..." What are you talking about, big? This issue is about returning the ability to increase and decrease font size used on a page, and this has been done.

Comment 133

[big]

Ok, then why can't i do this? I increase the text everytime i change an email to view it? I can change the hotmail text once and then when i open another mail it goes right back to the small size again. fixed?

Comment 134

[Roman R.]

It works for me with Yahoo Mail and Gmail (and all other sites I remember) but not with Hotmail. You should file a new issue requesting that the zoom factor be remembered across Hotmail messages.