Closed
Bug 1343196
Opened 8 years ago
Closed 8 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•8 years ago
|
status-firefox54:
affected → ---
Comment hidden (mozreview-request) |
Reporter | ||
Comment 2•8 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•8 years ago
|
||
Added in the recommended changes :)
Reporter | ||
Comment 6•8 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•8 years ago
|
Attachment #8842750 -
Attachment is obsolete: true
Assignee | ||
Comment 8•8 years ago
|
||
I think it's okay now :)
Assignee | ||
Updated•8 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•8 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•8 years ago
|
||
Why has my review request been discarded?
Comment hidden (mozreview-request) |
Reporter | ||
Comment 12•8 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•8 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•8 years ago
|
||
Got it, I'm still learning the process :)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 16•8 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•8 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•8 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•8 years ago
|
||
Done :D
Assignee | ||
Comment 21•8 years ago
|
||
Thank you so much for helping me work through this!
Comment 22•8 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•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 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
•