Closed Bug 1343196 Opened 7 years ago Closed 7 years ago

Remove border and background fallback styling from toolbar.css

Categories

(Toolkit :: Themes, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: dao, Assigned: barun.parruck, Mentored)

References

Details

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

Attachments

(1 file, 1 obsolete file)

In toolkit/themes/windows/global/toolbar.css, toolkit/themes/osx/global/toolbar.css and toolkit/themes/linux/global/toolbar.css, we can remove border and background declarations from rules that also set -moz-appearance, since -moz-appearance causes these borders and backgrounds not to be used anyway.
Comment on attachment 8842563 [details]
Bug 1343196 - Removes some redundant background and border declarations .

https://reviewboard.mozilla.org/r/116350/#review118030

::: toolkit/themes/windows/global/toolbar.css:15
(Diff revision 1)
>  
>  toolbox {
>    -moz-appearance: toolbox;
> -  background-color: -moz-Dialog;
> -  border-top: 2px solid;
>    -moz-border-top-colors: ThreeDShadow ThreeDHighlight;

-moz-border-* declarations can be removed too.

::: toolkit/themes/windows/global/toolbar.css:28
(Diff revision 1)
>  
>  toolbar {
>    min-width: 1px;
>    min-height: 19px;
>    border-top: 1px solid ThreeDHighlight;
>    border-bottom: 1px solid ThreeDShadow;

This isn't used either.

::: toolkit/themes/windows/global/toolbar.css:34
(Diff revision 1)
>  }
>  
>  toolbar:first-child, menubar {
>    min-width: 1px;
>    border-bottom: 1px solid ThreeDShadow;
>    border-top: 0px !important;

ditto
Attachment #8842563 - Flags: review-
Otherwise this patch looks good.
Assignee: nobody → barun.parruck
Added in the recommended changes :)
You seem to have committed the previous patch and this new one is against that commit. Instead we'll need all changes in one patch (diff against mozilla-central tip rather than on top of a local commit).
Attachment #8842750 - Attachment is obsolete: true
I think it's okay now :)
Attachment #8842750 - Attachment is obsolete: false
Attachment #8842750 - Attachment is patch: true
Attachment #8842750 - Attachment mime type: text/x-review-board-request → text/plain
Comment on attachment 8842750 [details]
Removes further redundant border declarations

>https://reviewboard.mozilla.org/r/116512/diff/#index_header
Attachment #8842750 - Attachment is obsolete: true
Attachment #8842750 - Attachment is patch: false
Why has my review request been discarded?
(In reply to Barun Parruck from comment #10)
> Why has my review request been discarded?

Likely because you didn't put "r?dao" in the commit message?
Comment on attachment 8842563 [details]
Bug 1343196 - Removes some redundant background and border declarations .

https://reviewboard.mozilla.org/r/116350/#review118228

::: toolkit/themes/windows/global/toolbar.css:39
(Diff revision 2)
>  menubar:-moz-lwtheme,
>  toolbox:-moz-lwtheme,
>  toolbar:-moz-lwtheme {
>    -moz-appearance: none;
>    background: none;
>    border-color: transparent;

I didn't see this earlier, but the above two lines can now be removed too.
Attachment #8842563 - Flags: review-
Got it, I'm still learning the process :)
Comment on attachment 8842563 [details]
Bug 1343196 - Removes some redundant background and border declarations .

https://reviewboard.mozilla.org/r/116350/#review118230

Looks good!
Attachment #8842563 - Flags: review?(dao+bmo) → review+
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 2 changes to 2 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev ee1fdb0b6fb3 needs "Bug N" or "No bug" in the commit message.
remote: Barun Parruck <barun.parruck@gmail.com>
remote: Bug #1343196 : Removes some redundant background and border declarations r=dao
remote: 
remote: MozReview-Commit-ID: Fiq04L00lBQ
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote
Can you please remove "#" from the commit message, replace ":" with "-" and add a dot or comma at the end before the review tag? Thanks!
Done :D
Thank you so much for helping me work through this!
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2492756574ea
Removes some redundant background and border declarations . r=dao
https://hg.mozilla.org/mozilla-central/rev/2492756574ea
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Blocks: 1344189
Depends on: 1389633
You need to log in before you can comment on or make changes to this bug.