Closed Bug 1027174 Opened 10 years ago Closed 8 years ago

Remove in-content preferences.css overrides now that the old prefs are backed out

Categories

(Firefox :: Settings UI, defect)

Product:
Firefox
Component:
Settings UI
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: Paenglab, Assigned: srivatsav1998, Mentored)

References

Details

(Whiteboard: [good first bug][lang=css])

Attachments

(2 files, 4 obsolete files)

bug-1027174-fix.patch
3.24 KB, patch
jaws
: review+
Details | Diff | Splinter Review
Bug 1027174 - Remove in-content preferences.css overrides now that the old prefs are backed out.
59 bytes, text/x-review-board-request
jaws
: review+
Details 
From bug 1020286 comment 7:

::: browser/themes/shared/incontentprefs/preferences.css
@@ +791,5 @@
> +}
> +
> +#noFxaAccount {
> +  /* override the preferences.css  margins */
> +  margin: 0;

We'll want to remove these overrides once we backout the old preference window implementation. Can you file a bug to remove the margins from the base preferences.css theme file so these aren't necessary.
Mentor: richard.marti, jaws
Summary: Remove in in-content preferences.css overrides when the old prefs are backed out. → Remove in-content preferences.css overrides when the old prefs are backed out.
Whiteboard: [good first bug][lang=css]
Hi Jared

So I just need to remove the margins from the preferences file? Does this apply just for #noFxaAccount?
Flags: needinfo?(jaws)
Comment on attachment 8694128 [details] [diff] [review]
I have the bug by overriding the margins from the base preferences.inc.css file

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

Thanks, I'll take a look at your patch.

::: browser/themes/shared/incontentprefs/preferences.inc.css
@@ -420,5 @@
>  #noFxaAccount {
>    /* Overriding the margins from the base preferences.css theme file.
>       These overrides can be simplified by fixing bug 1027174 */
> -  margin: 0;
> -  padding-top: 15px;

The padding-top here shouldn't be removed. It was added by bug 1193989, not bug 1020286 (which caused this current bug to get filed).

The comment about the override should be removed with the margin being removed.

This should be:

#noFxaAccount {
  padding-top: 15px;
}
Attachment #8694128 - Flags: review?(jaws) → review-
Assignee: nobody → baho
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Flags: needinfo?(jaws)
Attachment #8694338 - Flags: feedback-
Please make sure that you are testing your patch to see that there is no change in the appearance of the Sync page in the Firefox preferences (about:preferences) with and without your patch.

To fix this bug, you will need to remove the #noFxaAccount margin-related rules specified in:
> /browser/themes/linux/preferences/preferences.css
> /browser/themes/osx/preferences/preferences.css
> /browser/themes/windows/preferences/preferences.css

As well, you will need to remove the comment and margin-related rule specified in
> browser/themes/shared/incontentprefs/preferences.inc.css
like the patches in this bug are doing so far. Please do not change the padding-top rule that is next to the margin rules.
Not seeing activity for long time, so I'm clearing both "Assigned" statuses.
Assignee: baho → nobody
Status: ASSIGNED → NEW
Hi, i am attending an open source module at university, and would like to be assigned to this bug.
(In reply to Ganiyat Adebayo from comment #20)
> Hi, i am attending an open source module at university, and would like to be
> assigned to this bug.

Hi Ganiyat, thanks for your interest and apologies for not responding sooner. Please feel free to begin work on this bug. I will assign it to you when you upload a patch for it :)
Attachment #8694128 - Attachment is obsolete: true
Attachment #8694338 - Attachment is obsolete: true
Attachment #8697529 - Attachment is obsolete: true
Attachment #8697539 - Attachment is obsolete: true
Summary: Remove in-content preferences.css overrides when the old prefs are backed out. → Remove in-content preferences.css overrides now that the old prefs are backed out
Hey Jared, I would like to work on this bug. Will try to submit a patch soon.
Hey Jared, I would like to work on this bug. I am new to bug solving so please help me in solving this bug. I have removed the lines as you mentioned. Please verify it and help me further. Can you assign this bug to me?
Attachment #8823696 - Flags: review?(jaws)
Comment on attachment 8823696 [details] [diff] [review]
bug-1027174-fix.patch

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

This looks good but just the one issue below. I'll remove those changes and get this checked in for you. Thanks for your help and sorry for not responding sooner.

::: browser/config/mozconfig
@@ +1,2 @@
> +# Automatically download and use compiled C++ components:
> +ac_add_options --enable-artifact-builds

These changes should be removed.
Attachment #8823696 - Flags: review?(jaws) → review+
Assignee: nobody → srivatsav1998
Status: NEW → ASSIGNED
Comment on attachment 8825116 [details]
Bug 1027174 - Remove in-content preferences.css overrides now that the old prefs are backed out.

https://reviewboard.mozilla.org/r/103344/#review103926
Attachment #8825116 - Flags: review?(jaws) → review+
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/57add45987a7
Remove in-content preferences.css overrides now that the old prefs are backed out. r=jaws
https://hg.mozilla.org/mozilla-central/rev/57add45987a7
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: