Closed Bug 1163384 Opened 8 years ago Closed 8 years ago

Theming issues with the start / stop recording buttons on Linux


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



(firefox40 verified, firefox41 verified)

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


(Reporter: sjakthol, Assigned: sjakthol)



(Whiteboard: [polish-backlog])


(4 files)

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:
- 64bit:
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.

Assignee: nobody → sjakthol
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]

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+
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]

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.
Whiteboard: [devedition-40] → [polish-backlog]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.