1163384 - Theming issues with the start / stop recording buttons on Linux

Status: VERIFIED FIXED

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

Description

[Sami Jaktholm]

Attachment: performance-theme-issues-dark-theme.mp4

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

Comment 1

[Sami Jaktholm]

Attachment: performance-theme-issues-light-theme.mp4

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

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..

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Any yall able to check this out?

Comment 4

[Sami Jaktholm]

Attachment: bug-1163384-toolbarbutton-theming-issues.patch

Figured this out. The buttons need class 'devtools-toolbarbutton' to receive correct theming.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=af776c21e60e

Comment 5

[Sami Jaktholm]

Attachment: buttons-with-patch.png

Here's what the buttons look like with the patch.

Comment 6

[Brian Grinstead [:bgrins]]

Thanks Sami!

Comment 7

[Jordan Santell [:jsantell] [@jsantell]]

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!

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/914dda9f4a1a

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

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 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/914dda9f4a1a

Comment 11

[Sami Jaktholm]

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.

Comment 12

[Jordan Santell [:jsantell] [@jsantell]]

We'll also be uplifting all the perf tools patches for June 2nd, so may not be necessary for just this patch

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e3e6b0dd3bc6

Comment 14

[Petruta Horea [:phorea], Desktop Test Engineering]

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!

Comment 15

[Sami Jaktholm]

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.

Comment 16

[Petruta Horea [:phorea], Desktop Test Engineering]

No problem, thanks for the details.