396635 - "Launch Application" dialog clips buttons on bottom

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Al Billings [:abillings - ex-MoCo]]

The new "Launch Application" dialog is now clipping the "cancel" and "ok" buttons at its default size.

See attached image.

This was found in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007091704 Minefield/3.0a8pre.

Comment 1

[Al Billings [:abillings - ex-MoCo]]

Attachment: The dialog.

Comment 2

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

This was also mentioned in bug 389705, but I guess it isn't really quite the same bug. Might be fixed by a patch there, though.

Comment 3

[Joe Smith]

Attachment: Launch Application dialog has bottom and right cut-off

I've also got this running on Ubuntu 7.10 Beta and Gnome 2.20
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007100209 Minefield/3.0a9pre ID:2007100209

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Seems to also be mentioned in bug 394258.

Comment 5

[pal-moz]

dupe/similar bug 394711 ?

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #5)
> dupe/similar bug 394711 ?

Yours is a duplicate, but of which bug (and the other will be duped to _it_), I'm not sure... 

Comment 8

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

This basically does what Bug 389705 was going to do (that's targeting M10, this is M9, so let's fix it here).  If mfinkle could take a quick look-over, that'd be great.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 284811 [details] [diff] [review]
v1.0


>-<!ENTITY window.width "320">
>-<!ENTITY window.height "250">
>+<!ENTITY window.EMwidth "26em">
>+<!ENTITY window.EMheight "26em">

EMwidth & EMheight? not "-" worthy but I'd drop the "EM". We don't use "PX" prefixes

> <dialog id="handling"
>         ondialogaccept="return dialog.onAccept();"
>         onload="dialog.initialize();"
>-        width="&window.width;" height="&window.height;"
>+        style="min-width: &window.EMwidth;; min-height: &window.EMheight;;"
>         persist="width height screenX screenY sizemode"
>         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">

Why are we persisting "sizemode" ? I yanked it from my patch.

r=mfinkle with those answered

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

I'll remove the relevant changes from bug 389705, since they are handled here. Bug 389705 will still handle other stuff.

Comment 11

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

Comment on attachment 284811 [details] [diff] [review]
v1.0

I agree with mfinkle on both points - the names don't need "EM", and persisting sizemode doesn't make sense for a dialog.

Comment 12

[Shawn Wilsher :sdwilsh]

Alright, but it's my understanding that we need to be changing the name of strings if we change them.  I would think that this applies to this especially because we want this in em's now (and if a locale doesn't update it and had something similar to before, they'll have a really really big dialog).

Comment 13

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

(In reply to comment #12)
> Alright, but it's my understanding that we need to be changing the name of
> strings if we change them.

Ah, yes. Good point. I don't know how likely these are to have already been translated, but better to be safe than sorry.

Comment 14

[Shawn Wilsher :sdwilsh]

in that case, suggestions for the new name, or is this sufficient?

Comment 15

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

Those are fine.

Comment 16

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

Although, maybe emWidth to avoid it being confused with EM (Extension Manager)? Doesn't matter much either way :)

Comment 17

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

for checkin.

Addresses review comments.  This won't fix it for people who have already opened the window (because of the persists stuff), but any first timers to it and any new profiles will work out correctly.

This would be nice to have for the beta, and should be low risk.

Comment 19

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 286222 [details] [diff] [review]
v1.1

(Please leave the target milestone alone next time!)

Comment 20

[Shawn Wilsher :sdwilsh]

Checking in toolkit/locales/en-US/chrome/mozapps/handling/handling.dtd;
new revision: 1.2; previous revision: 1.1
Checking in toolkit/mozapps/handling/content/dialog.xul;
new revision: 1.4; previous revision: 1.3

Comment 21

[Dão Gottwald [:dao]]

I think min-width:&window.emWidth;em; or min-width:&window.styleWidth;; would have made more sense. Oh well ...

Comment 22

[Robert Kaiser]

hrm, using em for width is suboptimal, you probably should have used ch units instead, which really base on character width, while em bases on line height (and is fine for height because of that).

Comment 23

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007103004 Minefield/3.0a9pre and  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007103004 Minefield/3.0a9pre. I verified by launching some links from mail.

As a side note, when I use Thunderbird trunk on the Mac I get the dialog when the pref is set to ask. When I use Thunderbird branch, no matter what the pref is set to on trunk, I don't get the ask dialog. Not sure if this is expected, I can file a spinoff bug for this.