400466 - application selector dialog isn't wide enough if there is long file type description

Status: RESOLVED FIXED

(Firefox :: File Handling, defect, P3)

Description

[Vlado Valastiak [:wladow]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007101905 Minefield/3.0a9pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007101905 Minefield/3.0a9pre

Application selector dialog (bug 348808) isn't wide enough, when opening this dialog for the FIRST time for content type with long description. If you close and open it again for taht file type, width is correct.

Tested on Windows Vista with en-US and sk latest FX nightlies.

Reproducible: Always

Steps to Reproduce:
1. open Application tab in FX Options window 
2. select content type with long name 
3. select Choose application from drop-down menu
4. Dialog with wrong width is shown
5. click on Cancel button
6. select Choose application from drop-down menu again
7. Dialog with correct width is shown

Comment 1

[Vlado Valastiak [:wladow]]

Attachment: Helper application dialog showing for the first and second time

Comment 2

[Jim Mathies [:jimm]]

I guess maybe we should either wrap these or ellipse them. 

Myk can you give me some feedback on this? Also, weren't we thinking of rolling this dialog into something else for the new content handling? Maybe I can hold off on making any changes.

Comment 3

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

(In reply to comment #2)
> I guess maybe we should either wrap these or ellipse them. 
> 
> Myk can you give me some feedback on this? Also, weren't we thinking of rolling
> this dialog into something else for the new content handling? Maybe I can hold
> off on making any changes.

We are planning to consolidate the dialogs, but it isn't happening for Firefox 3, AFAIK.  cc:ing dmose for confirmation on that.

In the meantime, I think we should crop these long descriptions, adding ellipses at the end.  cc:ing faaborg for confirmation on that.

Comment 4

[Jim Mathies [:jimm]]

Attachment: screenshot - new clipping

Comment 5

[Jim Mathies [:jimm]]

Attachment: patch v.1

I went with center crop so users can see both the begining of the file and the  extension.

Comment 6

[Jim Mathies [:jimm]]

also I nixed the groove at the bottom of the dialog. You can view that here - 

https://bugzilla.mozilla.org/attachment.cgi?id=271748

Since I implemented this the groove has always stood out to me as something that didn't match our other content dialogs, so I took it out to clean things up.

Comment 7

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

Comment on attachment 287726 [details] [diff] [review]
patch v.1

>Index: toolkit/components/apppicker/content/appPicker.css

>+    width:250px !important;
>+    max-width:250px !important;
> }
> 
> #suggested-filename {
>     font-weight:normal;
>+    width:250px !important;
>+    max-width:250px !important;
> }

I don't think this is necessary.  I think the crop attribute by itself (or perhaps in conjunction with an align="stretch" attribute) should fix the problem.

Otherwise this looks fine, but I'm not able to test it, as I can't find a testcase that demonstrates the problem.  Do you have one you can point me at?

Comment 8

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

Comment on attachment 287724 [details]
screenshot - new clipping

Yup, centre clipping ftw!

Comment 9

[Jim Mathies [:jimm]]

I tried the crop attrib alone (and a few other combinations of attribs/css) but it didn't work, the dialog resized to the size of the longest text string. I'll try removing the widths and throw in the align to see what happens. If that doesn't do it though I'd be open to other suggestions.

Comment 10

[Jim Mathies [:jimm]]

Attachment: test file

Comment 11

[Jim Mathies [:jimm]]

Yeah that didn't work either. You either end up with a first time clipped dialog, or a dialgo that stretches out to the length of the text. The widths were the only way I was able to get around this. 

Comment 12

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

I'm still having trouble testing this.  With or without the patch, I see the same thing when I click the "test file" attachment: a left-cropped name.  It looks like I'm getting the unknowncontenttype.xul dialog instead of apppicker.xul, even though I'm on Windows XP.  Is there anything I need to do to enable the app picker?

Comment 13

[Jim Mathies [:jimm]]

When you get the unknown content dialog, click the dropdown and select choose (or browse?), that should bring up the app licker.

Comment 14

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

Comment on attachment 287726 [details] [diff] [review]
patch v.1

Ok, with help from dmose I found the dialog.  For some reason I was thinking it would pop up instead of the UCTH dialog instead of in addition to it once I click the Browse button.

(In reply to comment #11)
> Yeah that didn't work either. You either end up with a first time clipped
> dialog, or a dialgo that stretches out to the length of the text. The widths
> were the only way I was able to get around this. 

This is because the containing vbox isn't flexible.  If it were, it would expand to fill the space available to the right of the content icon, and the labels would crop themselves appropriately given only the crop attribute without the need to specify explicit widths.

r=myk with that change

Comment 15

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

Note: I filed bug 403982 on inconsistencies in style between the UCTH dialog and the app picker dialog.

Jim: can you add a newline to apppicker.xul when checking it in?

Comment 16

[Jim Mathies [:jimm]]

> This is because the containing vbox isn't flexible.

Ok, cool, that worked. Thanks Myk. Patch coming up...

> Jim: can you add a newline to apppicker.xul when checking it in?

added.

Comment 17

[Jim Mathies [:jimm]]

Attachment: patch v.2

Comment 18

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

Comment on attachment 289350 [details] [diff] [review]
patch v.2

Requesting approval for this very low-risk appearance fix.

Comment 19

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

Requesting wanted-firefox3 for this appearance glitch.

Comment 20

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

Checking in toolkit/components/apppicker/content/appPicker.xul;
/cvsroot/mozilla/toolkit/components/apppicker/content/appPicker.xul,v  <--  appPicker.xul
new revision: 1.2; previous revision: 1.1
done

Note: I checked this in without the align="stretch" change, which is unnecessary (I was mistaken earlier in suggesting it).