405133 - Implement UI for full page zoom (SeaMonkey part)

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, enhancement)

Description

[Jens Hatlak (:InvisibleSmiley)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.8.1.9) Gecko/20071030 SeaMonkey/1.1.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.8.1.9) Gecko/20071030 SeaMonkey/1.1.6

Filing this as per bug 389628 comment 18 and discussion in mozilla.dev.apps.seamonkey.

Options of implementing this include, but are not limited to:
a) simple pref
b) pref + UI
  i) option in Preferences
  ii) switch entry in Zoom menu to toggle full page / text zoom
  iii) two Zoom menus, one for full page and one for text zoom.

Reproducible: Always

Comment 1

[Caio Tiago Oliveira (asrail)]

I would prefer to use two menu items for that.

i) Adding one preference also increases complexity of interface
ii) A hidden preference in about:config is the FX way of doing that, we usually show it or display a preference on our preferences panel
iii) The user might want to mix both zooms for some reason

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #1)
> i) Adding one preference also increases complexity of interface

A bit maybe. But maybe the strongest argument against it is bad discoverability.

> ii) A hidden preference in about:config is the FX way of doing that, we usually
> show it or display a preference on our preferences panel

If your "ii)" refers to my "ii)" then you misunderstood me. Your "ii)" is equivalent to my "a)". What I suggest in "ii)" is a checkbox-style toggle inside the existing Text Zoom menu (which would have to be renamed to just "Zoom"). I agree with you that a simple pref for this is not enough in the SM world, still it is a possibility, so I listed it.

> iii) The user might want to mix both zooms for some reason

Might. I'm not sure if this theoretical possibility is enough to justify a second menu item which might confuse users. Maybe we need more people's opinions here.

Comment 3

[Caio Tiago Oliveira (asrail)]

(In reply to comment #2)
> (In reply to comment #1)
> > ii) A hidden preference in about:config is the FX way of doing that, we usually
> > show it or display a preference on our preferences panel
> 
> If your "ii)" refers to my "ii)" then you misunderstood me. Your "ii)" is
> equivalent to my "a)". What I suggest in "ii)" is a checkbox-style toggle
> inside the existing Text Zoom menu (which would have to be renamed to just
> "Zoom"). I agree with you that a simple pref for this is not enough in the SM
> world, still it is a possibility, so I listed it.

Yes...

but the average of users that don't understand both menus would be the same of those who don't understand the checkbox.

The issue here would be if they would understand the difference between text zoom and full zoom.

Two items would make it quicker to apply the kind of change he wants.

Some pages may get a better zoom on text zoom than full page zoom, so the expert user would like to be able to use full page zoom on some pages and text zoom on others.

Some expert users might prefer to use text zoom most of the time, but see a need to use full page zoom on places where it breaks the layout.

How simple would be to manage full page zoom on a tab and text zoom on another?

I would call two menus a advantage over FX.


Remember that the average users of SM are expert and corporative users.

;)

Comment 4

[Robert Kaiser]

The ii) option sounds very compelling to me, along with renaming the menu to "Zoom" and making full zoom the default.

Having two menus sounds like the worst way to do it for me, as that would only confuse people. Also, I think mixing the two is not something we want to provide through UI, it would only be confusing.

And actually, the last comment from asrail basically says "the normal user doesn't understand the difference but probably wants full zoom, some experts want to switch between both" - for me, the perfect case for going with ii) as we are targeting advanced users - if we'd target novies, the Firefox way would be correct (so we just say their decision is right for their audience) ;-)

Comment 5

[[e-mail]]

Attachment: Draft patch

This is just something I whipped up as a proof of concept. What do you think?

Comment 6

[Robert Kaiser]

(In reply to comment #5)
> This is just something I whipped up as a proof of concept. What do you think?

Looks like a good first step to me, the only thing we need to figure out in addition then is the switch itself

Comment 7

[[e-mail]]

Attachment: Part 1 - implement pref

Comment 8

[Andrew Schultz]

Comment on attachment 290053 [details] [diff] [review]
Part 1 - implement pref

>-      <menu id="menu_textZoom"/>
>+      <menu id="menu_zoom"/>

viewSource.xul and viewPartialSource.xul also need this.
navigator.js' hiddenWindowStartup() also needs s/text//.

Comment 9

[[e-mail]]

Attachment: Addressed review comments

View source uses gPrefs instead, so I added a pref member to the object.

Comment 10

[Andrew Schultz]

Comment on attachment 290515 [details] [diff] [review]
Addressed review comments

>Index: suite/browser/navigator.js
>+Components.classes["@mozilla.org/login-manager;1"].getService();

inadvertent change?

Comment 11

[Andrew Schultz]

Comment on attachment 290515 [details] [diff] [review]
Addressed review comments

r=ajschult without the login-manager line

Comment 12

[[e-mail]]

Backend pref patch checked in.

Comment 13

[Robert Kaiser]

Is something still open here? Note that FF just followed our way and implemented the same ideas in bug 401322.

Comment 14

[[e-mail]]

We've got no UI for the pref yet.

Comment 15

[Robert Kaiser]

OK, I'm trying to add it to the appearance pref panel in bug 411215, we might want to have it in the menu itself as well though, just as FF has it now and as we already discussed back when work on this was done - that latter part should probably still be done here.

Comment 16

[Robert Kaiser]

Do we still want to match Firefox and add this to the Zoom menu, or are we satisfied with the pref and the patches in here only?

Comment 17

[Jens Hatlak (:InvisibleSmiley)]

Note: FF always calls the menu item Zoom, we call it Zoom or Text Zoom depending on the pref. IMO if we add a toggle (technically: checkbox) we should stop dynamically renaming the menu item.

Comment 18

[Philip Chee]

> Do we still want to match Firefox and add this to the Zoom menu, or are we
> satisfied with the pref and the patches in here only?

I ported NoSquint to SeaMonkey. I had to do some ugly hacks since it overlays the View->Zoom menu items. Matching the Firefox UI would be helpful.

Comment 19

[Ian Neal]

So the zoom pref is now on Appearance->Content pref panel, is there anything within scope of this bug? I would say any other work is outside of scope of this bug and a new bug should be logged for those items of work.
Either way - 2.0 is feature complete, so minusing this bug.

Comment 20

[Philip Chee]

SeaMonkey 2.0 shipped long ago. Closing.