Closed
Bug 1343196
Opened 7 years ago
Closed 7 years ago
Remove border and background fallback styling from toolbar.css
Categories
(Toolkit :: Themes, defect)
Toolkit
Themes
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.
Reporter | ||
Updated•7 years ago
|
status-firefox54:
affected → ---
Comment hidden (mozreview-request) |
Reporter | ||
Comment 2•7 years 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-
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
Added in the recommended changes :)
Reporter | ||
Comment 6•7 years 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•7 years ago
|
Attachment #8842750 -
Attachment is obsolete: true
Assignee | ||
Comment 8•7 years ago
|
||
I think it's okay now :)
Assignee | ||
Updated•7 years 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•7 years 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•7 years ago
|
||
Why has my review request been discarded?
Comment hidden (mozreview-request) |
Reporter | ||
Comment 12•7 years 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•7 years 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•7 years ago
|
||
Got it, I'm still learning the process :)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 16•7 years 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•7 years 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•7 years 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•7 years ago
|
||
Done :D
Assignee | ||
Comment 21•7 years ago
|
||
Thank you so much for helping me work through this!
Comment 22•7 years 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2492756574ea
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•