498153 - Modern needs global/appPicker.css (The list in "Select helper application" dialog is higher than the dialog)

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[d]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090223 SeaMonkey/2.0a3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090223 SeaMonkey/2.0a3

The list in "Select helper application" dialog is higher than the dialog, and its bottom and the "browse..." button isn't shown.

Reproducible: Always

Steps to Reproduce:
1.open preferences->helper applications
2.click on the action list of any content type
3.click "use other..."
Actual Results:  
the list in the dialog is higher than the dialog

Expected Results:  
the list in the dialog should fit in the dialog

Comment 1

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1pre) Gecko/20090609 SeaMonkey/2.0b1pre] (comm-1.9.1-win32-unittest/1244589044) (W2Ksp4)
(http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c1a708160268
 +http://hg.mozilla.org/comm-central/rev/d1a9b79cf5c2)

The list is long and can be scrolled, as I would expect.

***

Can you attach a picture and give more details about how you would want it to look like?

Comment 2

[d]

Attachment: helper applications list

Comment 3

[Philip Chee]

Attachment: Patch 1.0 Just a copy of winstripe.

> +#file-info {
> +}
I have no idea why the winstripe file has all these empty styles.

> +#app-picker-item {
> +#app-picker-item-image {
> +#app-picker-item-cell {
Apparently these are generated by appPicker.js

Comment 4

[[e-mail]]

Comment on attachment 385581 [details] [diff] [review]
Patch 1.0 Just a copy of winstripe.

No jar.mn entry?

Should be a space after each :

>+#app-picker {
>+    width:320px !important;
Doesn't need to be important. Also, width should probably be a ch value.

>+    min-width:320px !important;
Don't need this.

>+#suggested-filename {
>+    font-weight:normal;
Unnecessary. (Might as well remove the empty rules too.)

>+#app-picker-list {
>+    height:225px;
Ideally this would fit 5 items exactly. The value depends on borders, which we don't currently have, but need at some point...

>+    min-height:225px;
Don't need this.

>+#app-picker-item {
>+    padding-bottom:5px;
>+    padding-top:5px;
>+}
>+
>+#app-picker-item-image {
>+}
>+
>+#app-picker-item-cell {
>+    font-weight:normal; 
>+    padding-right:10px;
>+    padding-left:10px;
>+}
I looked at these and it might be better to add a uniform 5px padding to the image and the text instead.

>+#browse-button {
>+    margin-top:10px;
Don't think we need this.

>\ No newline at end of file
Don't need this.

Comment 5

[Philip Chee]

Attachment: Patch 1.1 fixed review comments.

> No jar.mn entry?
Fixed.

> Should be a space after each :
Fixed.

>>+#app-picker {
>>+    width:320px !important;
> Doesn't need to be important.
Fixed.
> Also, width should probably be a ch value.
Fixed.
 
>>+    min-width:320px !important;
> Don't need this.
Fixed.

>>+#suggested-filename {
>>+    font-weight:normal;
> Unnecessary. (Might as well remove the empty rules too.)
Fixed. Fixed.

>>+#app-picker-list {
>>+    height:225px;
> Ideally this would fit 5 items exactly. The value depends on borders, which we
> don't currently have, but need at some point...
Fixed. Added border.

>>+    min-height:225px;
> Don't need this.
Fixed.

>>+#app-picker-item {
>>+    padding-bottom:5px;
>>+    padding-top:5px;
>>+}
>>+
>>+#app-picker-item-image {
>>+}
>>+
>>+#app-picker-item-cell {
>>+    font-weight:normal; 
>>+    padding-right:10px;
>>+    padding-left:10px;
>>+}
> I looked at these and it might be better to add a uniform 5px padding to the
> image and the text instead.
Fixed.

>>+#browse-button {
>>+    margin-top:10px;
>Don't think we need this.
Fixed.

> >\ No newline at end of file
> Don't need this.
Fixed.

Comment 6

[[e-mail]]

(In reply to comment #5)
>Created an attachment (id=385642)
>>>+#app-picker-list {
>>>+    height:225px;
>> Ideally this would fit 5 items exactly. The value depends on borders, which we
>>don't currently have, but need at some point...
>Fixed. Added border.
Actually it was the app-picker-list's border I was thinking of (although that's not to say that there shouldn't be a bottom border on the item).

Comment 7

[[e-mail]]

Comment on attachment 385642 [details] [diff] [review]
Patch 1.1 fixed review comments.

>+#app-picker-list {
>+  height: 222px;
r+sr=me with this changed to 224px to fit with the forthcoming 2px border.

Comment 8

[Philip Chee]

Attachment: Patch 1.2 fixed nit. r+sr=Neil

>>+#app-picker-list {
>>+  height: 222px;
> r+sr=me with this changed to 224px to fit with the forthcoming 2px border.
Fixed.

Comment 9

[Stefan [:stefanh]]

http://hg.mozilla.org/comm-central/rev/db3495388eb7