53207 - View -> Text size menu missing (re-enable it).

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P2)

Description

[Tobias Burnus]

Hi in 2000-09-18-21/Linux the View|Text size has disappeard. In the last
Thursday's version it was still there.
Just before weekend the the menu had been extended to use percentual values
instead of only tiny, large etc.

Now there is no item at all.

See also:
bug 37940: Include some absolute zoom levels in `Text Size' menu

Comment 1

[Peter "jag" Annema]

I'll take this. Putting it back depends on bug 52895 and blocks 37940. Changing
component to XP Apps: GUI Features.

Comment 2

[Peter "jag" Annema]

And accepting.

Comment 3

[John Morrison]

*** Bug 53879 has been marked as a duplicate of this bug. ***

Comment 4

[John Morrison]

Just a note in favour of getting this done on the NS6 branch (now that
we are branched). If we are planning to ship with this feature enabled
(and we *are* planning to do so), then it would be best to ship the
penultimate beta with this menu enabled, so that we exercise the
underlying layout code. That way, if it uncovers a problem with the
layout code for this feature, then we will have a chance to fix this
before RTM.

Turning this menu back on is known to be safe in and of itself, and was 
only disabled due to it triggering a problem in Mac menus.

Comment 5

[Peter "jag" Annema]

Copying over John's nsbeta3 and rtm nomination from bug 53879.

I'll attach the simple patch and go through the motions of getting this turned
back on on the trunk.

Also, there's bug 52969, in case bug 39117 won't get fixed in time.

Comment 6

[Peter "jag" Annema]

Attachment: [patch] Turn the text size menu back on

Comment 7

[Brendan Eich [:brendan]]

a=[e-mail] for the trunk.  Wait for PDT approval before committing to 
the branch.

/be

Comment 8

[John Morrison]

Jag has made this checkin on the trunk, I see. Thanks jag. Assigning to 
pinkerton to bring this up for consideration for checkin on the branch. 

Comment 9

[Mike Pinkerton (not reading bugmail)]

why can't jag land this on the branch?

Comment 10

[John Morrison]

I suppose jag could do that, but brendan's comment above was "wait for PDT
approval". I suppose we could just appeal directly to PDT for that approval,
but in the interim, I gave it to you, so Peter sees it, so it gets the magic
"+", and then can get the uber-magic "++" from PDT. (Just trying to figure 
out how this is all supposed to work ...).

Comment 11

[Peter "jag" Annema]

Adding regression keyword, since we had this feature in a different form
("Enlarge Text Size"/"Reduce Text Size") in nsbeta2.

Adding "Fix checked in on trunk" to status whiteboard.

Comment 12

[Peter Trudelle]

rtm+, but still needs info (r=?).  Regression of very useful functionality,
tested since before beta2, simple, safe fix tested on trunk for several days.
Missing r= should just be an overlooked formality. Please delete 'need info' in
status whiteboard when module owner (should be Pink in this case) gives it.

Comment 13

[Mike Pinkerton (not reading bugmail)]

ok, i tested it in my commercial build by applying the patch and it doesn't
freak out mac anymore, but as stated before, the functionality doesn't work.

r=pinkerton, but FWIW, I'm not the module owner of the contents of the menus.
That would be someone on don's team.

Comment 14

[Mike Pinkerton (not reading bugmail)]

to the owner go the spoils....

Comment 15

[don]

Oh what a thrill.  So I now own two RTM bugs with conflicting objectives, this
one and bug #52969.  Someone want to tell me whatinthehell is going on here?!?

I just checked with Paul Chen and he says that the menu is definitely is missing
on the Mac branch build.  It's obviously missing on the Windows branch builds.

Is it actually in on the trunk builds now?  Can someone confirm this?  And does
it work on any other platform?  I mean besides Mac.

And maybe can we close bug #52969 now?

Comment 16

[Peter "jag" Annema]

Okay, let's recap :-)

I checked in an enhanced version of this menu. It broke mac menus severely, so
it was commented out. When the mac menu bug was fixed, I uncommented it on the
trunk, now someone needs to uncomment it on the branch.

In the mean time bug 52969 was filed because the backend code called by this
doesn't work (yet) on Macs (bug 39117), so the request was to just not show this
on Mac at all. So bug 52969 isn't fixed yet, because when this one is fixed
(i.e. uncommented out) it will need to be turned off again for the Mac (either
through some build magic or runtime checks), unless bug 39117 is fixed first.

I'm not going to spend time on bug 52969 while people are still deciding what to
do with bug 39117 (or did I miss a resolution?), but in the mean time it would
be nice if the menu would just be turned on again on the branch so Windows and
Unix users can give it a beating, and Mac users can see what they're missing and
go vote for bug 39117 ;-)

Comment 17

[Akkana Peck]

Joining bug because this is dogfood for me: there are a lot of pages I can't
read on the branch due to text size, but can read fine on the trunk since I can
resize them.  I hope this covers the key bindings as well, since resizing text
is a very commonly needed operation.

Comment 18

[Phil Peterson]

PDT agrees [rtm need info] until super review is available.

Comment 19

[Phil Peterson]

Whoops, brendan already reviewed, so [rtm++]

Comment 20

[Peter "jag" Annema]

Will check this in on the branch tomorrow (unless someone else wants to?), and
now that bug 39117 got minused :-( I'll work on the "don't show this on Mac" fix
(bug 52969) later this week if/when I get time for it.

Comment 21

[don]

OK, we can only land this for Windows and Linux.

Peter, don't check in a fix unless you have a solution for bug #52969 first.
BTW, do you want to own these two bugs or do you want Ben to hold them?

Comment 22

[Peter "jag" Annema]

Taking this bug. Does Ben still need to bless it with his r=/a=?

Comment 23

[John Morrison]

Earlier today, you got an r=pinkerton, so I think that's covered. (FWIW, I
also tested this 'uncomment the menu' on Mac and it works for me). 

I don't believe that this needs to wait for a solution to bug 52969 (hide on
Mac, show it on win32/linux). It is safe to have this menu on Mac, it's just
that the back-end text resizing is a complete no-op. 

Comment 24

[Peter "jag" Annema]

Would you mind checking it in then? I gotta catch a few hours of sleep, being on
the hook prevents that ;-)

Comment 25

[John Morrison]

Sorry, I am not able to checkin as I have never filled out the appropriate
paperwork (and it keeps me out of trouble too :-]).

Comment 26

[don]

Yes, Ben still needs to bless this and the other bug.  And yes, I want the other
fix landed at the same time, if possible.  Hey, I'm a freakin' tyrant. :-)

Comment 27

[Peter "jag" Annema]

Making this depend on 52969 then. Patch available. *tap tap tap*

Comment 28

[Ben Goodger (use ben at mozilla dot org for email)]

a=ben

Comment 29

[Jason Eager]

*ALERT*: The PDT has inexplicitly marked blocking bug 52969 [RTM-].

I've removed the RTM- and added a detailed comment to that bug, but we might
need to make a phone call or two to get the bug rtm++'ed so we can finally get
this in.

Comment 30

[Jason Eager]

Removing Status Whiteboard comment about being blocked, since PDT has changed
status of the other bug back to rtm++.

Comment 31

[Peter "jag" Annema]

Fix checked in on branch, marking fixed.

Comment 32

[sairuh (rarely reading bugmail)]

vrfy fixed on the branch on linux and winnt [2000.10.11.09-n6 opt comm bits].
[moot for mac due to bug 39117 and bug 52969.]

Comment 33

[Asa Dotzler [:asa]]

Verified Fixed on trunk builds. Menus are there and functional
linux 101808 RedHat 6.2
win32 101804 NT 4
Setting bug to Verified and removing vtrunk keyword