458601 - port the full zoom UI to thunderbird

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

In bug 389628 (and some followups) full zoom UI was implemented for firefox. I think we should follow suit.

That is, change the View > Text Size to be

 Zoom >    Zoom In
           Zoom Out
        -----------------
           Reset
        -----------------
        x  Zoom Text Only      

I'm not sure if we want browser.zoom.full true or not... (this patch doesn't change it from the default false that is false).

Firefox also has per-site zooming (as a hidden pref), the part for handling that I just ripped out.

Also in this patch:
 - noticed we were including mailCore.js twice (since it's now in baseMenuOverlay too)
 - removed a few unused entities (reloadCmd.label and friends)

Comment 1

[Elmar Ludwig]

If per-site zooming is not used, you could probably drop all of the FullZoom controller except for FullZoom.updateMenu() and directly call the ZoomManager from the oncommand handlers, I think.

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

You're right, the previous patch still had some unnecessary code.

Comment 3

[Phil Ringnalda (:philor)]

Comment on attachment 341834 [details] [diff] [review]
proposed fix, v2

Looks good, and having seen just how many many times people who don't like full zoom can comment in bugs saying over and over "we hates it, we does," clearly defaulting to text zoom is the right answer ;)

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 341834 [details] [diff] [review]
proposed fix, v2

This looks good over all.  

I'm wondering if there is a better way to phrase the Text Only Zoom.  Maybe by saying "Zoom Message Text Only", however that doesn't really hit it either.  

I'm assuming that the only other thing we can actually zoom is inline images.  And I can't think of an easy way to convey that so maybe it's just fine as is.

Comment 5

[Magnus Melin [:mkmelin]]

Well, not only images, but also other elements with absolute width (px not em or %).

Fix checked in.
changeset:   548:2ef2829f4950
http://hg.mozilla.org/comm-central/rev/2ef2829f4950

->FIXED

Comment 6

[Vlado Valastiak [:wladow]]

Comment on attachment 341834 [details] [diff] [review]
proposed fix, v2

>-<!ENTITY reloadCmd.label "Reload">
>-<!ENTITY reloadCmd.accesskey "R">
>-<!ENTITY stopCmd.label "Stop">
>-<!ENTITY stopCmd.accesskey "S">

Are you sure you wanted to remove these?

Comment 7

[Magnus Melin [:mkmelin]]

Pretty sure they were unused, yes.

Comment 8

[Phil Ringnalda (:philor)]

Only since 2002-10-04 (revision 1.3 of mailWindowOverlay.xul, where we stopped having them below the Display Attachments Inline menuitem in the View menu) :)

Comment 9

[alta88]

(In reply to comment #0)

> Firefox also has per-site zooming (as a hidden pref), the part for handling
> that I just ripped out.

i really wish you had left that for rss feed web page viewing, i encounter the problem of needing different zoom levels quite often among feeds.

Comment 10

[Phil Ringnalda (:philor)]

Since the Fx implementation would just be remembering the mailbox:/// URI, I suspect that would be surprising for people who mix feeds in a single folder, and very surprising for people who zoomed one message in their Inbox and got them all zoomed. A separate bug that only remembered for feeds, and remembered the iframed URI instead of the mailbox:/// one, might be nice, though.

Comment 11

[alta88]

yes, clearly the value of content-base (a uri) would be used for rss (likely stored as the domain being sufficient, even for different feeds from the same aggregator case).  and it would also be nice if the 'from' in pop/imap or 'newsgroup' for news could do the same.  imo, this would be little new code and things like this make for slickness in an app.

filed bug 458948.

Comment 12

[Peter Lairo]

Remember zoom level on a per sender basis would be cool.