The default bug view has changed. See this FAQ.

Remove border and background fallback styling from toolbar.css

RESOLVED FIXED in Firefox 54

Status

()

Toolkit
Themes
RESOLVED FIXED
28 days ago
24 days ago

People

(Reporter: dao, Assigned: Barun Parruck, Mentored)

Tracking

(Blocks: 1 bug, {good-first-bug})

Trunk
mozilla54
good-first-bug
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

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

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

28 days ago
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.
(Reporter)

Updated

28 days ago
status-firefox54: affected → ---
Comment hidden (mozreview-request)
(Reporter)

Comment 2

27 days ago
mozreview-review
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-
(Reporter)

Comment 3

27 days ago
Otherwise this patch looks good.
Assignee: nobody → barun.parruck
Comment hidden (mozreview-request)
(Assignee)

Comment 5

27 days ago
Added in the recommended changes :)
(Reporter)

Comment 6

27 days ago
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).
Comment hidden (mozreview-request)
(Assignee)

Updated

26 days ago
Attachment #8842750 - Attachment is obsolete: true
(Assignee)

Comment 8

26 days ago
I think it's okay now :)
(Assignee)

Updated

26 days ago
Attachment #8842750 - Attachment is obsolete: false
Attachment #8842750 - Attachment is patch: true
Attachment #8842750 - Attachment mime type: text/x-review-board-request → text/plain
(Assignee)

Comment 9

26 days ago
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
(Assignee)

Comment 10

26 days ago
Why has my review request been discarded?
Comment hidden (mozreview-request)
(Reporter)

Comment 12

26 days ago
(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?
(Reporter)

Comment 13

26 days ago
mozreview-review
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-
(Assignee)

Comment 14

26 days ago
Got it, I'm still learning the process :)
Comment hidden (mozreview-request)
(Reporter)

Comment 16

26 days ago
mozreview-review
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+

Comment 17

26 days ago
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
(Reporter)

Comment 18

26 days ago
Can you please remove "#" from the commit message, replace ":" with "-" and add a dot or comma at the end before the review tag? Thanks!
Comment hidden (mozreview-request)
(Assignee)

Comment 20

26 days ago
Done :D
(Assignee)

Comment 21

26 days ago
Thank you so much for helping me work through this!

Comment 22

26 days ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2492756574ea
Removes some redundant background and border declarations . r=dao

Comment 23

26 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2492756574ea
Status: NEW → RESOLVED
Last Resolved: 26 days ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
(Reporter)

Updated

25 days ago
Blocks: 1344189
You need to log in before you can comment on or make changes to this bug.