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

RESOLVED FIXED in seamonkey2.0b1

Status

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: dmdmdm, Assigned: philip.chee)

Tracking

({modern})

Trunk
seamonkey2.0b1
modern
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
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
[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?
Version: unspecified → Trunk
(Reporter)

Comment 2

9 years ago
Created attachment 383227 [details]
helper applications list
(Assignee)

Updated

9 years ago
Blocks: 456757
Severity: major → normal
Status: UNCONFIRMED → NEW
Component: Download & File Handling → Themes
Ever confirmed: true
Keywords: modern
OS: Windows XP → All
QA Contact: download → themes
Hardware: x86 → All
Summary: The list in "Select helper application" dialog is higher than the dialog → Modern needs global/appPicker.css (The list in "Select helper application" dialog is higher than the dialog)
(Assignee)

Comment 3

9 years ago
Created attachment 385581 [details] [diff] [review]
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
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #385581 - Flags: review?(neil)

Comment 4

9 years ago
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.
Attachment #385581 - Flags: review?(neil) → review-
(Assignee)

Comment 5

9 years ago
Created attachment 385642 [details] [diff] [review]
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.
Attachment #385581 - Attachment is obsolete: true
Attachment #385642 - Flags: superreview?(neil)
Attachment #385642 - Flags: review?(neil)

Comment 6

9 years ago
(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).
(Assignee)

Updated

9 years ago
Depends on: 465924

Updated

9 years ago
Attachment #385642 - Flags: superreview?(neil)
Attachment #385642 - Flags: superreview+
Attachment #385642 - Flags: review?(neil)
Attachment #385642 - Flags: review+

Comment 7

9 years ago
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.
(Assignee)

Comment 8

9 years ago
Created attachment 385745 [details] [diff] [review]
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.
Attachment #385642 - Attachment is obsolete: true
Attachment #385745 - Flags: superreview+
Attachment #385745 - Flags: review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [patch for checkin comment 8]

Comment 9

9 years ago
http://hg.mozilla.org/comm-central/rev/db3495388eb7
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [patch for checkin comment 8]
Target Milestone: --- → seamonkey2.0b1

Updated

9 years ago
Attachment #385745 - Attachment description: [for checkin] Patch 1.2 fixed nit. r+sr=Neil → Patch 1.2 fixed nit. r+sr=Neil
(Assignee)

Updated

9 years ago
Blocks: 465924
No longer depends on: 465924
You need to log in before you can comment on or make changes to this bug.