1300473 - The checked attribute is gone on .recording-button-mount .record-button in Performance

Status: VERIFIED FIXED

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

Description

[magicp]

Attachment: the-checked-attribute-is-gone.png

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160904030201

Steps to reproduce:

1. Start Nightly
2. Go to any sites (e.g. about:home, bugzilla.mozilla.org)
3. Open DevTools > Performance (Shift+F5)
4. Start Recording
5. Check button style of .recording-button-mount .record-button


Actual results:

.recording-button-mount .record-button is not change to checked style for recording, because the checked attribute is gone.

Regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1789229965bfc5e7b08dfcf1c054c366abe1267a&tochange=d61bbdd0b155f76170adef0e9433e2166c606483


Expected results:

.recording-button-mount .record-button has checked attribute.

Comment 1

[Tim Nguyen :ntim]

Attachment: Properly set recording buttons classes in performance tool

Comment 2

[Tim Nguyen :ntim]

So basically, the patch:
- Uses a checked class instead of an attribute (the new console front-end does that)
- Uses the CSS :disabled state to lock the button
- Fixes the reported issue
- Adds testcases for the reported issue

Comment 3

[Patrick Brosset <:pbro>]

Comment on attachment 8789684 [details] [diff] [review]
Properly set recording buttons classes in performance tool

Review of attachment 8789684 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good from a high level, but I'd rather let Greg perform the last review. Sorry I know I said I could do it for you quickly, but I've been busy today, and Greg will be back Monday, I think it's better if he looks at it (he has blocked review requests for now though, so I can't just transfer the review).

Comment 4

[Greg Tatum [:gregtatum]]

Comment on attachment 8789684 [details] [diff] [review]
Properly set recording buttons classes in performance tool

Review of attachment 8789684 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great! Thanks for taking care of this. This is a much cleaner solution.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/5b64b2e8e518
Properly set recording buttons classes in performance tool. r=gregtatum

Comment 6

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

Backed out for failing devtools test browser_perf-recording-selected-03.js:

https://hg.mozilla.org/integration/fx-team/rev/54cf088080cb11a9532f7b86ee1a77ed7e7bf89d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=5b64b2e8e518c80e39c53aa4872e80661bd8834d
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=11538560&repo=fx-team

16:23:22     INFO -  Waiting for event: 'Performance:UI:StateChanged' on [object PerformanceView] for 1 time(s) with arguments: {"1":{}}.
16:23:22     INFO -  Waiting for event: 'Performance:UI:OverviewRendered' on [object OverviewView] for 1 time(s) with arguments: {"1":100}.
16:23:23     INFO -  Ignoring event 'Performance:UI:StateChanged' with unexpected argument at index 1: recorded
16:23:23     INFO -  Event: 'Performance:UI:StateChanged' on [object PerformanceView] received.
16:23:23     INFO -  Event: 'Performance:RecordingStateChange' on [object PerformanceController] received.
16:23:23     INFO -  Event: 'Performance:BackendReadyRecordingStart' on [object PerformanceController] received.
16:23:23     INFO -  Event: 'Performance:UI:OverviewRendered' on [object OverviewView] received.
16:23:23     INFO -  Waiting for event: 'Performance:RecordingSelected' on [object PerformanceController] for 1 time(s).
16:23:23     INFO -  Event: 'Performance:RecordingSelected' on [object PerformanceController] received.
16:23:23     INFO -  TEST-INFO | started process screentopng
16:23:23     INFO -  TEST-INFO | screentopng: exit 0
16:23:23     INFO -  149 INFO checking window state
16:23:23     INFO -  150 INFO Entering test bound
16:23:23     INFO -  151 INFO Selecting recording #0 and waiting for it to be displayed.
16:23:23     INFO -  152 INFO TEST-UNEXPECTED-FAIL | devtools/client/performance/test/browser_perf-recording-selected-03.js | Button is still checked after selecting another item. -
16:23:23     INFO -  Stack trace:
16:23:23     INFO -      chrome://mochitests/content/browser/devtools/client/performance/test/browser_perf-recording-selected-03.js:null:35
16:23:23     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:784:9
16:23:23     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:704:7
16:23:23     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:743:59

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/68be9a522185
Properly set recording buttons classes in performance tool. r=gregtatum

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/68be9a522185

Comment 9

[magicp]

verified in 50 (Build ID 20160915030417). Thanks all.

Comment 10

[magicp]

(In reply to magicp from comment #9)
> verified in 50 (Build ID 20160915030417). Thanks all.
Oops. verified in 51 (Build ID 20160915030417). Thanks all.

Comment 11

[Tim Nguyen :ntim]

Thanks!