Closed Bug 1163384 Opened 8 years ago Closed 8 years ago

Theming issues with the start / stop recording buttons on Linux

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect, P1)

x86_64
Linux
defect

Tracking

(firefox40 verified, firefox41 verified)

VERIFIED FIXED
Firefox 41
Tracking Status
firefox40 --- verified
firefox41 --- verified

People

(Reporter: sjakthol, Assigned: sjakthol)

References

Details

(Whiteboard: [polish-backlog])

Attachments

(4 files)

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..
Blocks: perf-tool-v2
Priority: -- → P1
Any yall able to check this out?
Flags: needinfo?(vporof)
Flags: needinfo?(bgrinstead)
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)
Attached image buttons-with-patch.png
Here's what the buttons look like with the patch.
Thanks Sami!
Flags: needinfo?(vporof)
Flags: needinfo?(bgrinstead)
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+
https://hg.mozilla.org/mozilla-central/rev/914dda9f4a1a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-40] → [devedition-40]
Target Milestone: --- → Firefox 41
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+
Flags: qe-verify+
No longer blocks: perf-40-uplifts
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)
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)
No problem, thanks for the details.
Status: RESOLVED → VERIFIED
Whiteboard: [devedition-40] → [polish-backlog]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.