[Modern] Too much padding in prefwindows

RESOLVED FIXED in seamonkey2.0a1

Status

SeaMonkey
Preferences
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: stefanh, Assigned: stefanh)

Tracking

Trunk
seamonkey2.0a1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 2 obsolete attachments)

(Assignee)

Description

11 years ago
The prefwindows in Modern has way too much padding. This will break certain panes. See https://bugzilla.mozilla.org/attachment.cgi?id=296695 in bug 411620 for an example.
(Assignee)

Comment 1

11 years ago
Created attachment 296818 [details] [diff] [review]
Fix padding issues

OK, so afaics the prefwindow's padding should always be the same. And we don't want any padding on the buttons.
(Assignee)

Comment 2

11 years ago
Comment on attachment 296818 [details] [diff] [review]
Fix padding issues

Philip said he would test this and see if it breaks any extensions.

Comment 3

11 years ago
Created attachment 296916 [details]
InfoLister <prefwindow> old. vs new.

Unfortunately the tabstrip acquires an ugly and unwanted border. I'll be attaching some screenshots.

Comment 4

11 years ago
Created attachment 296917 [details]
DownThemAll <prefwindow> old. vs new.

Comment 5

11 years ago
Created attachment 296918 [details]
Scrapbook <prefwindow> old. vs new. (tabstrip not used)

Comment 6

11 years ago
Created attachment 296919 [details]
Raw unedited screenshots of the above.
For scrapbook (attachment #296918 [details], comment #5) I don't see any difference. For the other two, the grey line at top doesn't bother me much though I can understand it would some people. 

Now what did the patch increase by about that many? The padding-bottom (3rd of 4 parts in "padding") at communicator/preferences line 44 doesn't seem a likely candidate so it might be the padding-top which used to be 0 for prefwindow but gets the former value (7px) from prefpane at suite/themes/modern/global/preferences.css line 47. Would the scrapbook prefwindow become unacceptably ugly if we brought it back to 0px? If it does, is there an acceptable substitute (such as reestablishing a prefpane padding-top, maybe not as large as 7px)?

Comment 8

11 years ago
We can't change the CSS in global preferences.css because that will break extensions, so we need to override the CSS in communicator preferences.css as if the prefwindow was a child prefwindow.

Note that there are some nits with the global rules if you want to fix them:
the prefpane has bottom padding, which I think it shouldn't
the six rules don't look like they're in a consistent order
the second .prefWindow-dlgbuttons rule doesn't use a child selector

Comment 9

11 years ago
(In reply to Comment 8)
> the prefpane has bottom padding, which I think it shouldn't
> the six rules don't look like they're in a consistent order
> the second .prefWindow-dlgbuttons rule doesn't use a child selector

You didn't mention these nits when you gave the sr+ in Bug 390270 :P
(Assignee)

Comment 10

11 years ago
> the second .prefWindow-dlgbuttons rule doesn't use a child selector

Is there any reason we can't just use the class all by itself?

Comment 11

11 years ago
(In reply to comment #10)
>>the second .prefWindow-dlgbuttons rule doesn't use a child selector
>Is there any reason we can't just use the class all by itself?
We'd have to tweak prefwindow.xml to xbl:inherit the xpfe attribute.

Comment 12

11 years ago
In fact, we'll also have to tweak prefwindow.xml so that prefpanes get styled by the communicator version of preferences.css as they don't currently.
(Assignee)

Comment 13

11 years ago
Created attachment 296953 [details] [diff] [review]
Fix panel padding, take 2

So, this should leave all the (not tested) extension dialogs untouched. I also re-arranged the style rules in global/preferences.css and the pane now have 0px bottom margin.
Attachment #296818 - Attachment is obsolete: true
Attachment #296953 - Flags: superreview?(neil)
Attachment #296953 - Flags: review?(neil)

Comment 14

11 years ago
>+.prefWindow-dlgbuttons,
>+prefwindow[type="child"] > .prefWindow-dlgbuttons {
>+  padding: 0px;
>+}
>+
>+.prefWindow-dlgbuttons {
>+  padding: 0px 5px 5px 5px;
>+}
>

So what /should/ the padding be on |.prefWindow-dlgbuttons| ?

>+prefwindow[type="child"] > .prefWindow-dlgbuttons {
>+  padding: 0px;
>+}

Too much cut and paste?
(Assignee)

Comment 15

11 years ago
Comment on attachment 296953 [details] [diff] [review]
Fix panel padding, take 2

(In reply to comment #14)

> Too much cut and paste?

Ugh. Very much so.
Attachment #296953 - Attachment is obsolete: true
Attachment #296953 - Flags: superreview?(neil)
Attachment #296953 - Flags: review?(neil)
(Assignee)

Comment 16

11 years ago
Created attachment 296988 [details] [diff] [review]
Fix padding, take 3

OK, this one should actually be the right one...
Attachment #296988 - Flags: superreview?(neil)
Attachment #296988 - Flags: review?(neil)

Comment 17

11 years ago
Comment on attachment 296988 [details] [diff] [review]
Fix padding, take 3

>+      <xul:hbox xbl:inherits="type" anonid="dlg-buttons" class="prefWindow-dlgbuttons" pack="end">
Nit: xbl:inherits normally goes last, or at least after anonid and class.
Attachment #296988 - Flags: superreview?(neil)
Attachment #296988 - Flags: superreview+
Attachment #296988 - Flags: review?(neil)
Attachment #296988 - Flags: review+
(Assignee)

Comment 18

11 years ago
Landed with xbl:inherits at the end:

Checking in suite/common/bindings/prefwindow.xml;
/cvsroot/mozilla/suite/common/bindings/prefwindow.xml,v  <--  prefwindow.xml
new revision: 1.7; previous revision: 1.6
done
Checking in suite/themes/modern/communicator/preferences.css;
/cvsroot/mozilla/suite/themes/modern/communicator/preferences.css,v  <--  preferences.css
new revision: 1.2; previous revision: 1.1
done
Checking in suite/themes/modern/global/preferences.css;
/cvsroot/mozilla/suite/themes/modern/global/preferences.css,v  <--  preferences.css
new revision: 1.2; previous revision: 1.1
done

Comment 19

11 years ago
I did some testing with the DownThemAll options window in the DOM Inspector. I noticed something when I set type="child" on the prefwindow element.

>Index: suite/common/bindings/prefwindow.xml
......
>-      <xul:hbox anonid="dlg-buttons" class="prefWindow-dlgbuttons" pack="end">
>+      <xul:hbox xbl:inherits="type" anonid="dlg-buttons" class="prefWindow-dlgbuttons" pack="end">
......
>Index: suite/themes/modern/global/preferences.css
......
>-prefwindow[type="child"] .prefWindow-dlgbuttons {
>+.prefWindow-dlgbuttons[type="child"] {
>   padding: 0px;
> }

The problem with this is that extensions will use the toolkit <prefwindow> widget and not the communicator one so |.prefWindow-dlgbuttons[type="child"]| doesn't work.

Comment 20

11 years ago
I reverted that change, thanks.

Comment 21

11 years ago
(In reply to Comment 20)
> I reverted that change, thanks.

Confirmed extension prefwindow dialog box now has the expected padding for type="child".
(Assignee)

Comment 22

11 years ago
Thanks. And sorry for the mess I made here.
(Assignee)

Comment 23

11 years ago
--> Fixed
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0alpha
You need to log in before you can comment on or make changes to this bug.