Closed Bug 1163384 Opened 8 years ago Closed 8 years ago
Theming issues with the start / stop recording buttons on Linux
564.25 KB, video/mp4
650.99 KB, video/mp4
2.12 KB, patch
|Details | Diff | Splinter Review|
13.94 KB, image/png
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
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..
Priority: -- → P1
Any yall able to check this out?
Figured this out. The buttons need class 'devtools-toolbarbutton' to receive correct theming. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=af776c21e60e
Assignee: nobody → sjakthol
Status: NEW → ASSIGNED
Attachment #8604076 - Flags: review?(jsantell)
Here's what the buttons look like with the patch.
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+
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
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?
We'll also be uplifting all the perf tools patches for June 2nd, so may not be necessary for just this patch
Attachment #8604076 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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!
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.
No problem, thanks for the details.
Whiteboard: [devedition-40] → [polish-backlog]
You need to log in before you can comment on or make changes to this bug.