1428093 - Mac preferences subdialog button left/right margins 12px instead of 0

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

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

Attachment: remove left/right margins around subdialog buttons on Mac

In the process of scriptifying preferences, I converted .prefWindow-dlgbuttons CSS rules to their .dialog-button-box equivalents.  And on Mac I also reconciled conflicting rules setting the left/right margins for the dialog-button-box.

But I reconciled them incorrectly, retaining the rule that sets those margins to 12px, whereas the rule that sets them to 0 is the one that should have been retained.  So dialog-button-box now has a different margin on Mac.  This patch fixes up that visual regression.

In the process of fixing this regression, I also realized that some of the rules I converted are no longer necessary, as there are no longer any "parent" preferences dialogs; all the remaining preferences dialogs are "children" (i.e. subdialogs).

So this patch also replaces the rules that set padding for .dialog-button-box — and then override it for |.prefwindow[type="child"] .dialog-button-box — in favor of a single rule that sets padding for .dialog-button-box in subdialogs.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8939890 [details] [diff] [review]
remove left/right margins around subdialog buttons on Mac

Review of attachment 8939890 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/osx/preferences/in-content/dialog.css
@@ +14,5 @@
>  }
> +
> +.dialog-button-box {
> +  margin: 0;
> +  padding-top: 0 !important;

Are we sure we still need the !important here?

Comment 2

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

Attachment: remove left/right margins around subdialog buttons on Mac

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> Comment on attachment 8939890 [details] [diff] [review]
> remove left/right margins around subdialog buttons on Mac
> 
> Review of attachment 8939890 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/osx/preferences/in-content/dialog.css
> @@ +14,5 @@
> >  }
> > +
> > +.dialog-button-box {
> > +  margin: 0;
> > +  padding-top: 0 !important;
> 
> Are we sure we still need the !important here?

Good question!  I just checked, and we don't need the !important there, which also means we don't need this rule at all, since the padding: 0 rule in browser/themes/shared/incontentprefs/dialog.inc.css sets padding-top to this value.

Then I checked the margin: 0 rule, and it turns out that we don't need that one either, since 0 is the default computed margin of the box.

So I've removed both rules from this version of the patch, which is the version I'll push inbound.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/de578768ff3b
remove left/right margins around subdialog buttons on Mac; r=jaws

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/de578768ff3b

Comment 5

[Dão Gottwald [:dao]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #2)
> Then I checked the margin: 0 rule, and it turns out that we don't need that
> one either, since 0 is the default computed margin of the box.

Same for padding, or was there some other rule setting a non-zero padding?

Comment 6

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

Attachment: remove unnecessary dialog-button-box padding

(In reply to Dão Gottwald [::dao] from comment #5)
> (In reply to Myk Melez [:myk] [@mykmelez] from comment #2)
> > Then I checked the margin: 0 rule, and it turns out that we don't need that
> > one either, since 0 is the default computed margin of the box.
> 
> Same for padding, or was there some other rule setting a non-zero padding?

Same for padding, it looks like.  I'm not sure why I thought the padding rule was still needed, but there isn't any other rule that sets padding for that box, on any platform.

(It makes me wonder what other rules we have that are obsolete or redundant.)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/858276d87233
remove unnecessary dialog-button-box padding; r=dao

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/858276d87233