Closed Bug 1013718 Opened 7 years ago Closed 7 years ago

in-content preferences: advanced pane: alignment issue 'cached web content'

Categories

(Firefox :: Preferences, defect)

All
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 32
Tracking Status
firefox32 --- verified

People

(Reporter: soeren.hentzschel, Assigned: Paenglab)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

in-content preferences: advanced pane. Please see the attached screenshot for the alignment issue.
Blocks: 738796
Attached patch alignCache.patch (obsolete) — Splinter Review
Add margin to align.
Assignee: nobody → richard.marti
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8426970 - Flags: review?(jaws)
In this patch you add a 16px left margin. In the patch for bug 1013709 you add a 10px left padding. Why the difference? Can these be shared somehow?
Flags: needinfo?(richard.marti)
The two elements in #setDefaultPane (the other bug), #setDefaultButton and description have a left-margin and #useCacheBefore from this bug not. This makes the difference. Something like:

#setDefaultPane,
#useCacheBefore {
  -moz-margin-start: 10px; /* align with checkbox-label above */
}

#setDefaultPane > description,
#setDefaultPane > #setDefaultButton {
  -moz-margin-start: 0;
}

would work. Would you prefer this?
Flags: needinfo?(richard.marti)
Attached patch alignIndent.patch (obsolete) — Splinter Review
This patch gives the indent class a indent of 39px and aligns also the checkboxes correctly. With changing the defaultLabel from description to label the margin is automatically 0px.

This patch fixes also bug 1013709.
Attachment #8426970 - Attachment is obsolete: true
Attachment #8426970 - Flags: review?(jaws)
Attachment #8427290 - Flags: review?(jaws)
Comment on attachment 8427290 [details] [diff] [review]
alignIndent.patch

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

This is really close. I verified the 39px amount on Windows. Did you check that this was the right size for other platforms too or do you need me to do that?

::: browser/themes/shared/incontentprefs/preferences.css
@@ +912,5 @@
> +}
> +
> +.indent .indent checkbox,
> +.indent .indent > *:first-child:not(.indent) {
> +  -moz-margin-start: -4px !important;

*:first-child:not(.indent) is really expensive and isn't necessary here if you move class="indent" from the <hbox> down to the <button> and <label>
Attachment #8427290 - Flags: review?(jaws) → feedback+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> Comment on attachment 8427290 [details] [diff] [review]
> alignIndent.patch
> 
> Review of attachment 8427290 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is really close. I verified the 39px amount on Windows. Did you check
> that this was the right size for other platforms too or do you need me to do
> that?

Good point, I was to optimistic all platforms where equal here. But now they are. ;)

The indent is now 33px on all platforms.

> ::: browser/themes/shared/incontentprefs/preferences.css
> @@ +912,5 @@
> > +}
> > +
> > +.indent .indent checkbox,
> > +.indent .indent > *:first-child:not(.indent) {
> > +  -moz-margin-start: -4px !important;
> 
> *:first-child:not(.indent) is really expensive and isn't necessary here if
> you move class="indent" from the <hbox> down to the <button> and <label>

This is now no more needed.
Attachment #8427290 - Attachment is obsolete: true
Attachment #8430365 - Flags: review?(jaws)
Comment on attachment 8430365 [details] [diff] [review]
alignIndent.patch

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

::: browser/themes/osx/preferences/in-content/preferences.css
@@ +19,5 @@
>    -moz-margin-start: 0;
>  }
>  
> +.checkbox-icon {
> +  margin-right: 0;

This and...

@@ +23,5 @@
> +  margin-right: 0;
> +}
> +
> +.radio-icon {
> +  -moz-margin-end: 0;

this are maybe looking weird but I used the same properties as in toolkit are used.
Comment on attachment 8430365 [details] [diff] [review]
alignIndent.patch

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

Looks good, thanks.
Attachment #8430365 - Flags: review?(jaws) → review+
Keywords: checkin-needed
Duplicate of this bug: 1017631
https://hg.mozilla.org/mozilla-central/rev/8056d7248fb0
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
Verified fixed on Mac OSX 10.8.5 using latest Nightly 32.0a1 (buildID: 20140609030202).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.