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.

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

Comment on attachment 296818 [details] [diff] [review] Fix padding issues Philip said he would test this and see if it breaks any extensions.

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

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)?

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

(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

> 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?

(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.

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.

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.

>+.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?

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.

OK, this one should actually be the right one...

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.

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

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.

I reverted that change, thanks.

(In reply to Comment 20) > I reverted that change, thanks. Confirmed extension prefwindow dialog box now has the expected padding for type="child".

Thanks. And sorry for the mess I made here.

--> Fixed