Closed
Bug 1163384
Opened 9 years ago
Closed 9 years ago
Theming issues with the start / stop recording buttons on Linux
Categories
(DevTools :: Performance Tools (Profiler/Timeline), defect, P1)
Tracking
(firefox40 verified, firefox41 verified)
VERIFIED
FIXED
Firefox 41
People
(Reporter: sjakthol, Assigned: sjakthol)
References
Details
(Whiteboard: [polish-backlog])
Attachments
(4 files)
564.25 KB,
video/mp4
|
Details | |
650.99 KB,
video/mp4
|
Details | |
2.12 KB,
patch
|
jsantell
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
13.94 KB,
image/png
|
Details |
STR: 1. Have a Linux system. 2. Have a Linux build with fx-team rev 6c0788228680 in it (links at the end). 3. Open performance tools. 4. Start and stop recordings with both UI buttons and console.profile. 5. Switch themes and do #4 again. Expected results: The buttons should look like in attachment 8603680 [details] of bug 1082695. Actual results: * Light theme: the 'Stop recording' button text is white and the background is light gray making the text hard to read especially if the cursor is over the button. * Dark theme: - 'Start recording' text is dark gray against theme default background. - 'Start recording' background turns to light gray when hovered. - 'Stop recording' button is light gray with white text. See the attached screencasts for better illustration about the issues. Builds to test this with (from rev 6c0788228680 in Treeherder): - 32bit: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-linux/1431203525/firefox-40.0a1.en-US.linux-i686.tar.bz2 - 64bit: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-team-linux64/1431203525/firefox-40.0a1.en-US.linux-x86_64.tar.bz2
Assignee | ||
Comment 1•9 years ago
|
||
Comment 2•9 years ago
|
||
Looks like the `toolbarbutton.record-button` styles from bug 1082695 are not applying -- for any state. Not sure why this is but this should definitely try to make it to Fx40 on Tuesday..
Blocks: perf-tool-v2
Priority: -- → P1
Comment 3•9 years ago
|
||
Any yall able to check this out?
Flags: needinfo?(vporof)
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 4•9 years ago
|
||
Figured this out. The buttons need class 'devtools-toolbarbutton' to receive correct theming. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=af776c21e60e
Assignee | ||
Comment 5•9 years ago
|
||
Here's what the buttons look like with the patch.
Comment 7•9 years ago
|
||
Comment on attachment 8604076 [details] [diff] [review] bug-1163384-toolbarbutton-theming-issues.patch Review of attachment 8604076 [details] [diff] [review]: ----------------------------------------------------------------- Looks and works great! I thought I removed this intentionally because it caused some issue by overriding styles on OSX, but not seeing that now, so must've been wrong. Thanks, Sami!
Attachment #8604076 -
Flags: review?(jsantell) → review+
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
remote: https://hg.mozilla.org/integration/fx-team/rev/7f901174dd44 remote: https://hg.mozilla.org/integration/fx-team/rev/32ad078a4b5f remote: https://hg.mozilla.org/integration/fx-team/rev/914dda9f4a1a
Whiteboard: [fixed-in-fx-team][devedition-40]
https://hg.mozilla.org/mozilla-central/rev/914dda9f4a1a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-40] → [devedition-40]
Target Milestone: --- → Firefox 41
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8604076 [details] [diff] [review] bug-1163384-toolbarbutton-theming-issues.patch Requesting approval for aurora per comment 2. Approval Request Comment [Feature/regressing bug #]: Regression from bug 1082695. [User impact if declined]: Two buttons in the shiny, new profiler look out of place and contain hard to read text due to incorrect theming on Linux systems. [Describe test coverage new/current, TreeHerder]: Manual. I've checked this on Windows and Linux and Jordan checked it on OSX. [Risks and why]: A class was added to a button. It could cause the button to receive incorrect styles and if some code is querying elements with the generic class name 'devtools-toolbarbutton' it might apply any operations it performs on the matching elements to the buttons too. [String/UUID change made/needed]: None.
Attachment #8604076 -
Flags: approval-mozilla-aurora?
Comment 12•9 years ago
|
||
We'll also be uplifting all the perf tools patches for June 2nd, so may not be necessary for just this patch
Updated•9 years ago
|
Blocks: perf-40-uplifts
Updated•9 years ago
|
status-firefox40:
--- → affected
Updated•9 years ago
|
Attachment #8604076 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Flags: qe-verify+
Updated•9 years ago
|
No longer blocks: perf-40-uplifts
Comment 14•9 years ago
|
||
I couldn't reproduce the initial issue because the builds from comment 0 are no longer available. However, in both Firefox 40 beta 4 and latest Developer Edition 2015-07-16 under Ubuntu 14.04 LTS 32-bit, the Start/Stop Recording Performance buttons look good in dark and light theme. Sami, can I mark this as verified based on this and comment 11? Thank you!
Flags: needinfo?(sjakthol)
Assignee | ||
Comment 15•9 years ago
|
||
Yes, I think so. The buttons are now correctly themed also on Linux. I'd mark this bug verified but I don't know how to do it. So I'll have to leave it to you. Sorry for the inconvenience.
Flags: needinfo?(sjakthol)
Comment 16•9 years ago
|
||
No problem, thanks for the details.
Updated•9 years ago
|
Whiteboard: [devedition-40] → [polish-backlog]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•