419612 - pref to not update site-specific zoom for existing background tabs

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

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

Some folks (f.e. bug 386835, comment 9) have suggested that we not update the zoom for existing background tabs when a site-specific setting changes.  Instead, they suggest we only update the zoom when the sites in those tabs change, i.e. when the user browses to a different site in the current tab or opens a new tab to some site.

This seems like a reasonable behavior that some power users might want (f.e. Mozilla developers who want to load one tinderbox page at one zoom level and another one at another level), although I doubt it's the behavior we should expose by default (or even with a visible pref), and it leads to a bunch of tricky questions:

If you browse to a different site and then hit the Back button, should we update the zoom or preserve the previous zoom?

Should we preserve the zoom across sessions (i.e. hook it into the session store)?

Which zoom level is the site-specific one (the last one you picked, some previous selection, some separately-selected value)?

If a page in a background tab reloads itself, do we update the zoom?  What if it redirects the browser to another site with an updated pref?

Overall this seems complicated enough that I'm not sure it can happen for Firefox 3, but I'm reporting it to get it on file.  In the meantime, I have also filed bug 419609 on a hidden pref to disable per-site zoom entirely for users for whom the old behavior is better than the new one.

Comment 2

[Alex North-Keys]

Drop the whole textzoom-OTHER tabs/windows thing by default entirely, it interferes with a number of valid use cases, as several have already noted. There's a fundamental perceptual clash in whether the existing zoom mechanism applies to the webpage, the tab, or the whole browser.  It's complicated further by different multiwindow/monowindow usage patterns.  For me, I'd expect it to apply to only the current webpage/tab pair, and only to be preserved on webpages ALREADY rendered in that single tab's back/next list, and only for that session (unless restored through the Session Manager).

So my expectations seem to match what FF 2 implemented pretty well.  Only tab/page duples were zoomed, and at zoom there was at least a chance that the same midpage text would still be visible afterwards.  We were only missing a UI mechanism to set retained default zoom levels - we DID have a painful way to do it manually.

There was (is?) a userChrome.css hook for user-specified URI-specific CSS overrides. I'd like to see a menu option to record the current text-zoom, automatcally-converted into CSS form, into that mechanism. Ideally there would be a dialog for selecting either that one webpage, a whole site, or for all matching some regexp be affected, although I'm not sure if the userChrome.css mechanism already has a regexp function.

Certainly we'd be better off making the userChrome.css more visible instead of creating a unneeded new mechanism for retaining zoom settings.

Comment 4

[NLC]

Hi I'm new here.  Initially I thought that this was caused by opening a link in a new tab; the new tab would obtain the zoom level from the original tab.  I thought that it was a great feature.  Consider changing the site-specific nature of the zoom level to tab-specific in nature.

As mentioned in Bug 431332, "Can be disabled with browser.zoom.siteSpecific".

Comment 5

[NLC]

Sorry, I meant Bug 431322.

Comment 9

[Eric A. Seiden]

I posted a new bug early this morning (443635) because I didn't find this ticket. I was assuming it was a font rendering issue. But you may want to look at my example on that ticket to see why the FF2 behaviour was FAR better than the new FF3 behaviour. You could make it user selectable but that's a lot of work. If it were up to me (HAH!) I'd just fix it back to the old way.

Comment 12

[William]

Regards comment #2 ...

While I believe that the way FF v2 works is spot-on ...

I believe that that Zoom function is formatting option,  not a presentation layer option.

As a formatting option, it would be a tab-window only parameter; not a site or web-page level option.

In addition; if a user needed to enable a zoom-like feature for accessibility reasons (as a user option) that should be on the OPTIONS dialogue.  In my view of this, a "zoom function" is for zoom in/out as with an image or map.  If I wanted a blanket zoom-in or a blanket zoom-out, that would imply a browser setting.

As an aside, it might be a good idea to offer Web Site Options (or web site config) for those pesky web pages with 8pt and 7pt fonts every place.  :-D  That (again) is not a "zoom function" in my opinion.  Just a way to overcome really badly designed sites.  *grin*

Thanks, w.
         

Comment 13

[Manish Jethani]

I'm going to answer those tricky questions based on what I think as an end user:

>If you browse to a different site and then hit the Back button, should we update the zoom or preserve the previous zoom?

Ideally preserve the previous zoom. i.e. the same page loaded in the same tab should get the same zoom level it got the first time, unless the page has been manually reloaded (either by entering the URL or hitting the Reload button).

>Should we preserve the zoom across sessions (i.e. hook it into the session
store)?

Ideally, yes, but not that important.

>Which zoom level is the site-specific one (the last one you picked, some
previous selection, some separately-selected value)?

The last one you picked. Let's say you open multiple windows with different pages from the same site (something I do a lot with Wikipedia, by the way), the site-specific zoom level should be the one set last.

>If a page in a background tab reloads itself, do we update the zoom?  What if
it redirects the browser to another site with an updated pref?

The zoom level for a page should be updated only if the page has been reloaded manually, not if it has been reloaded programmatically or as a result of using the Back/Forward button.

If you get redirected to a page from another site with an updated site-specific zoom level, the page should get displayed with the new zoom level.

Comment 14

[Manish Jethani]

Again, speaking as an end user, what would be really cool is if this site-specific zoom feature was dropped in favour of Firefox 2 behaviour combined with the following.

When a user changes the zoom level on a page, she has the option to:

 1.  Lock the zoom level for that specific page.
 2.  Lock the zoom level for "all pages on this site" (i.e. by host name).

I don't know how feasible this would be to implement, but I'd prefer this any day over the current default site-specific behaviour. (Effectively what we have now is option #2 above applied to all sites by default.)

Comment 18

[Maël Leclair]

Hi,

For your information, from about:config, you can change the default value of browser.zoom.siteSpecific to false, which may fix some of your problems. (I got frustrated myself until I found it ;-) )

Cheers,
Maël.

Comment 19

[Eric A. Seiden]

Yes, that works QUITE like a charm. I believe they just need a checkbox in preferences to turn it on/off. I think most users are FAR too timid to go mucking about in the about:config file. 

And almost everyone I speak with DESPISES the new behaviour.

Comment 21

[Ocie Hudson]

For what it is worth, as a person who has visual acuity problems, it is better for me that Firefox continuously retain the zoom level that I've selected, for each and every page that is displayed (regardless of window, tab or website) at least until I exit the browser.  Magnifiers, whether software or hardware, just don't suffice.

It could be okay to change the current behavior (FF 3.0.4) to match one or more of the other suggestions, but ONLY if you guys are FIRST willing and able to correct the implementation of the "Minimum font size" option in Tools > Options > Content > Fonts & Colors: Advanced > Fonts.  As it is currently implemented in FF 3.0.4, using that option to set even a relatively small font size distorts the display of text on many pages of the same website (but not necessarily all pages) and many pages of other websites.  If I set the Minimum to a large size (14 or 16) then large sections of many pages will be overlaid by other parts of the same page, completely hiding whole parts of the page.  (Note: what I've described happens when the option "Allow pages to choose fonts ..." is enabled.)

So far, I haven't encountered a website for which the Fonts & Colors default font and size that I've specified appeared to be used.  But I suspect that selecting a large size for the font could create rendering errors, depending upon whether the text is commingled with images and/or Javascript buttons, etc.

Last, but not least, a similar rendering problem exists for the View > Zoom > Zoom Text Only option, so that it is infeasible to use it.  When the entire page is enlarged or diminished by just using the normal > Zoom Out and/or > Zoom In, then I haven't seen any problems with the display of the text.  (It is as if the whole page were treated as an image.)

I haven't found a report about this yet, but I will post one if I don't find one first.

Comment 24

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

Now that 386835 has landed, this should be as simple as just preffing the call to FullZoom.onLocationChange(gBrowser.currentURI); in browser.js' onUpdateCurrentBrowser.

Comment 25

[Nochum Sossonko [:Natch]]

Attachment: Patch ver. 1

Is this ok, or do you want the pref caching and updating in browser.js too?

Comment 26

[Nochum Sossonko [:Natch]]

Attachment: Patch ver. 1.1

Forgot to remove the observer in destroy.

Comment 27

[Nochum Sossonko [:Natch]]

Comment on attachment 357654 [details] [diff] [review]
Patch ver. 1.1

No need for two observers, gonna fix that shortly. Thanks Gavin

Comment 28

[Nochum Sossonko [:Natch]]

Attachment: Patch ver. 1.2

Only one observer.

Comment 29

[Nochum Sossonko [:Natch]]

Attachment: whoops, forgot to qrefresh

Sorry about that, same as before.

Comment 31

[Nochum Sossonko [:Natch]]

gavin: ping? This might be good to get into 3.1...

Comment 32

[kalyan]

1. Zoom settings must be preserved across sessions, I don't want to zoom every time I open my browser.

2. Zoom feature should be something very tab specific, but not site specific. Even it can taken out. If the user wants a new site he will open it in a new tab.

User wants to zoom a site, let him do it in a tab and then, he wants a new site, let him open it in a new tab. Forget what site he opens in that tab. If the browser again asks, "You are going away from this site to want to keep your zoom settings" that would be quite annoying. 

The plain old understanding of zoom is the best. Power users and also people who are acquinted with it will face difficulties.

Comment 33

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

(In reply to comment #32)
> 2. Zoom feature should be something very tab specific, but not site specific.

You can already set browser.zoom.siteSpecific to false to get that behavior. It isn't related to this bug.

Comment 34

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

Comment on attachment 357750 [details] [diff] [review]
whoops, forgot to qrefresh

Sorry for the delay...

Ideally the pref branch itself would also be rooted at browser.zoom, but I suppose that's a slightly more invasive change.

It's kind of odd to keep track of this pref in the FullZoom object given that it only affects browser.js behavior, but I suppose there's no point in trying to keep these two "modules" independent, especially since this dependence is mostly superficial, and removing it would probably involve code duplication.

Comment 35

[Nochum Sossonko [:Natch]]

Right, and it would need another observer in browser.js (which in general I think is large enough already), plus some more startup code...

Thanks for the review!

Comment 36

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/d80e72b94e49

Comment 37

[Shawn Wilsher :sdwilsh]

any reason this didn't land with a test?

Comment 38

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

It didn't land with a test because I hate testing (and freedom).

Nochum: could we get a test written to ensure both pref values work as expected? browser/base/content/test/browser_bug386835.js might be useful as inspiration.

Comment 39

[Nochum Sossonko [:Natch]]

Attachment: test

Sorry about that. This might be tested already indirectly in the background image tests or the tests Mossop landed for the progress listener changes, it's safe to say that the zoom feature probably has the most extensive browser-chrome tests in the tree :)

Comment 40

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

Thanks for the test!

Comment 41

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/17abd3beeabf

Comment 42

[Boris Zbarsky [:bzbarsky]]

Backed out; the push this was part of seems to be causing random leak orange on Mac.

Comment 43

[Boris Zbarsky [:bzbarsky]]

Looks like the random leak orange is still happening, so this can reland as desired...

Comment 44

[Boris Zbarsky [:bzbarsky]]

Actually, scratch that.  This orange is on Linux, whereas the orange from the push this landed as part of was on Mac.  I recommend landing this separately from the other 4 patches that landed with it.

Comment 45

[Dão Gottwald [:dao]]

The leak exists on 1.9.1, so this didn't cause it.

Comment 46

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/7a43f9939882

Comment 47

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

Comment on attachment 357750 [details] [diff] [review]
whoops, forgot to qrefresh

a191=beltzner for this and the test, plz be sure to land both

Comment 48

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d22bdbe06b40
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a7ee6f5878c6

Comment 50

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P5 - Polish issue that is rarely encountered, and is not easily identifiable.

Comment 51

[William]

It is extremely EASY to reproduce.

  1. Visit a web site (any page on that site)
  2. Use Zoom-IN or Zoom-OUT
  3. Visit a different web site, new domain URL, etc.
  4. Return to the first site, but select a different page.

  5. The second page on the same site, is still LARGE or SMALL 
    (as selected in step #2).

The bug is reproduced.

The frequency depends on the frequency the Zoom-IN and Zoom-OUT functions are used.  

This is a major USER-CENTER behaviour problem really because the code is forcing every page on a site to be rendered differently to the design behaviour.  

To me the Zoom is about me reading the phone book and I want to zoom-in on the fine print with a magnifier.  It is just that 'page', for "normal processing" I just want the normal view.  

I had this happen today, a site I must have visited a few weeks ago, didn't quite fit on the screen.  It took me quite some time to realise that it must be "enlarged" by a ZOOM-IN, before I realised I could see the whole page on my screen by going back to the non-ZOOM view.

I really think this discussion needs to go before a "user design" group, not coders.

AND it isn't FIXED at all, someone just decided they prefer it "this way"!

Does that mean every page on your phone book should be ZOOM-IN if you used a magnifier ONCE???  

best of luck convincing the printer.

w.

Comment 52

[Dão Gottwald [:dao]]

(In reply to comment #51)
What you're saying is unrelated to this bug. You want browser.zoom.siteSpecific=false.

Comment 54

[Alex Faaborg [:faaborg] (Firefox UX)]

>>I really think this discussion needs to go before a "user design" group, not
>>coders.
>>
>>AND it isn't FIXED at all, someone just decided they prefer it "this way"!

>What you're saying is unrelated to this bug. You want
>browser.zoom.siteSpecific=false.

We believe the decision to lock the preference to Web sites instead of Web pages makes sense, since it is more likely that you will want to view an entire site with the modification (like wikipedia, a particular blog that uses a small font, etc) as opposed to having to use the zoom control for ever single individual page you are reading.

Also, while the UX group doesn't currently do any implementation work ourselves, many of our front end engineers are well very well versed in interface design.

Comment 56

[Stefan]

"browser.zoom.siteSpecific" should be off by default. Even making it a visible option in the pref dialog would just unnecessarily clutter that.

A per tab zoom factor, plus inheritance of the zoom factor from the page that opened a new tab, is the only sensible and intuitive default behaviour I can think of.