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

VERIFIED FIXED in Firefox 51

Status

defect
VERIFIED FIXED
3 years ago
Last year

People

(Reporter: magicp.jp, Assigned: ntim)

Tracking

({regression})

Trunk
Firefox 51
Dependency tree / graph

Firefox Tracking Flags

(firefox51 verified)

Details

Attachments

(2 attachments)

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.
Blocks: 1297784
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Developer Tools: Performance Tools (Profiler/Timeline)
OS: Unspecified → All
Hardware: Unspecified → All
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED
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 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).
Attachment #8789684 - Flags: review?(pbrosset)
Attachment #8789684 - Flags: review?(gtatum)
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.
Attachment #8789684 - Flags: review?(gtatum) → review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/5b64b2e8e518
Properly set recording buttons classes in performance tool. r=gregtatum
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
Flags: needinfo?(ntim.bugs)
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/68be9a522185
Properly set recording buttons classes in performance tool. r=gregtatum
Flags: needinfo?(ntim.bugs)
https://hg.mozilla.org/mozilla-central/rev/68be9a522185
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
verified in 50 (Build ID 20160915030417). Thanks all.
(In reply to magicp from comment #9)
> verified in 50 (Build ID 20160915030417). Thanks all.
Oops. verified in 51 (Build ID 20160915030417). Thanks all.
Thanks!
Status: RESOLVED → VERIFIED
Depends on: 1327996
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.