Closed
Bug 1402290
Opened 7 years ago
Closed 7 years ago
Notification bar text is vertically misaligned
Categories
(Firefox :: Theme, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 58
People
(Reporter: roxana.leitan, Assigned: daleharvey)
References
(Blocks 1 open bug)
Details
(Whiteboard: [reserve-photon-visual])
Attachments
(4 files)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 Build ID: 20170921220243 [Affected versions]: Nightly 57.0a1 [Affected platforms]: Windows 10 x64, Ubuntu 16.04 x64, Mac OS X 10.12 [Steps to reproduce]: 1.Launch Nightly 57.0a1 with a new profile. 2.Install an ad blocker add-on 3.Navigate to http://www.popuptest.com/popuptest1.html [Expected result]: The notification bar should match the specifications: https://mozilla.invisionapp.com/share/ZKBC94BPQ#/screens/229252114 [Actual result]: The notification bar does not match the specs. (please see the attachment)
Updated•7 years ago
|
Whiteboard: [photon-visual] → [photon-visual] [triage]
In this attachment can be observed the differences between Expected and Actual results: - notification icon is different - Learn more button is missing - One "button" is missing - Different text size
Updated•7 years ago
|
Flags: qe-verify+
Priority: -- → P3
QA Contact: roxana.leitan
Whiteboard: [photon-visual] [triage] → [reserve-photon-visual]
Updated•7 years ago
|
Blocks: photon-visual
Priority: P3 → P4
Summary: Notification bar doesn't match the specifications → Notification bar text is vertically misaligned
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dharvey
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
I wasnt sure where to put the .notification-button styling, inside notification.inc.css it doesnt have any effect on the button, notification > button is whats giving that the current styling but didnt want to change it globally
Updated•7 years ago
|
Status: NEW → ASSIGNED
Priority: P4 → P1
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8916575 [details] Bug 1402290 - Update notification alignment. https://reviewboard.mozilla.org/r/187714/#review193042 ::: toolkit/themes/linux/global/global.css:280 (Diff revision 1) > > +.notification-button { > + border-radius: 0; > + padding: 6px 10px; > + margin-inline-end: 12px; > +} This looks completely out of place and I'm not even sure what problem you're trying to fix here. Please revert.
Attachment #8916575 -
Flags: review?(dao+bmo) → review-
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
I wasnt sure whether to only fix the alignment or to try to get it to look like the spec, right now the buttons are rounded and bar is much smaller which the notification-button changes fixed
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8916575 [details] Bug 1402290 - Update notification alignment. https://reviewboard.mozilla.org/r/187714/#review193062 ::: toolkit/themes/shared/notification.inc.css:8 (Diff revision 2) > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"); > > notification { > padding: 2px 0 3px; Your earlier patch evened out this padding, which made sense to me. Why did you revert this?
Assignee | ||
Comment 8•7 years ago
|
||
> Your earlier patch evened out this padding, which made sense to me. Why did you revert this?
Its fairly subtle but without the extra height if felt like the description was too low with even padding, with 2px the cap height (N) is exactly centered, but the ascenders are taller than the cap height so I think it can go either way, is your call
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8916575 [details] Bug 1402290 - Update notification alignment. https://reviewboard.mozilla.org/r/187714/#review193140
Attachment #8916575 -
Flags: review?(dao+bmo) → review+
Comment 11•7 years ago
|
||
Pushed by dharvey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/54c3651423b2 Update notification alignment. r=dao
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8916575 [details] Bug 1402290 - Update notification alignment. Approval Request Comment [Feature/Bug causing the regression]: Photon polish [User impact if declined]: Slightly misaligned text [Is this code covered by automated tests?]: nope [Has the fix been verified in Nightly?]: not yet [Needs manual test from QE? If yes, steps to reproduce]: [List of other uplifts needed for the feature/fix]: [Is the change risky?]: nope [Why is the change risky/not risky?]: very minor css change to text alignment [String changes made/needed]:
Attachment #8916575 -
Flags: approval-mozilla-beta?
![]() |
||
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/54c3651423b2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
status-firefox57:
--- → affected
Comment on attachment 8916575 [details] Bug 1402290 - Update notification alignment. Photon polish, Beta57+
Attachment #8916575 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/2220c5f79afb
Comment 16•7 years ago
|
||
Verified fixed in 57.0b9 on Windows 10 x64, macOS 10.13.1 and Ubuntu 16.04 x64.
Reporter | ||
Comment 17•7 years ago
|
||
Verified fixed using the latest Nightly 58.0a1 (2017-10-18) on Windows 10 x64, macOS 10.12 and Ubuntu 16.04 x64.
You need to log in
before you can comment on or make changes to this bug.
Description
•