Bug 1300473 - Properly set recording buttons classes in performance tool. r=gregtatum
authorTim Nguyen <ntim.bugs@gmail.com>
Mon, 12 Sep 2016 17:31:51 +0200
changeset 313535 68be9a5221854bcb29884f648bcebc706125e943
parent 313534 b8cba97b26485efbeab6530cefa75bc0a3365c0f
child 313536 b1156b0eb96fcb49966b20e5fcf6a01f634ea2ee
child 313544 7826ad7df05fb73b79f808885ed4ddea9210da55
push id30691
push userkwierso@gmail.com
push dateTue, 13 Sep 2016 00:19:42 +0000
treeherdermozilla-central@b1156b0eb96f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgregtatum
bugs1300473
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1300473 - Properly set recording buttons classes in performance tool. r=gregtatum
devtools/client/performance/components/recording-button.js
devtools/client/performance/components/recording-controls.js
devtools/client/performance/test/browser_perf-button-states.js
devtools/client/performance/test/browser_perf-recording-selected-03.js
devtools/client/themes/common.css
devtools/client/themes/performance.css
--- a/devtools/client/performance/components/recording-button.js
+++ b/devtools/client/performance/components/recording-button.js
@@ -8,22 +8,30 @@ const {DOM, createClass} = require("devt
 const {button} = DOM;
 
 module.exports = createClass({
   displayName: "Recording Button",
 
   render() {
     let {
       onRecordButtonClick,
-      isRecording
+      isRecording,
+      isLocked
     } = this.props;
 
+    let classList = ["devtools-button", "record-button"];
+
+    if (isRecording) {
+      classList.push("checked");
+    }
+
     return button(
       {
-        className: "devtools-toolbarbutton record-button",
+        className: classList.join(" "),
         onClick: onRecordButtonClick,
         "data-standalone": "true",
         "data-text-only": "true",
+        disabled: isLocked
       },
       isRecording ? L10N.getStr("recordings.stop") : L10N.getStr("recordings.start")
     );
   }
 });
--- a/devtools/client/performance/components/recording-controls.js
+++ b/devtools/client/performance/components/recording-controls.js
@@ -5,62 +5,46 @@
 
 const {L10N} = require("devtools/client/performance/modules/global");
 const {DOM, createClass} = require("devtools/client/shared/vendor/react");
 const {div, button} = DOM;
 
 module.exports = createClass({
   displayName: "Recording Controls",
 
-  /**
-   * Manually handle the "checked" and "locked" attributes, as the DOM element won't
-   * change by just by changing the checked attribute through React.
-   */
-  componentDidUpdate() {
-    if (this.props.isRecording) {
-      this._recordButton.setAttribute("checked", true);
-    } else {
-      this._recordButton.removeAttribute("checked");
-    }
-
-    if (this.props.isLocked) {
-      this._recordButton.setAttribute("locked", true);
-    } else {
-      this._recordButton.removeAttribute("locked");
-    }
-  },
-
   render() {
     let {
       onClearButtonClick,
       onRecordButtonClick,
       onImportButtonClick,
       isRecording,
       isLocked
     } = this.props;
 
+    let recordButtonClassList = ["devtools-button", "record-button"];
+
+    if (isRecording) {
+      recordButtonClassList.push("checked");
+    }
+
     return (
       div({ className: "devtools-toolbar" },
         div({ className: "toolbar-group" },
           button({
             id: "clear-button",
             className: "devtools-button",
             title: L10N.getStr("recordings.clear.tooltip"),
             onClick: onClearButtonClick
           }),
           button({
             id: "main-record-button",
-            className: "devtools-button record-button",
+            className: recordButtonClassList.join(" "),
+            disabled: isLocked,
             title: L10N.getStr("recordings.start.tooltip"),
-            onClick: onRecordButtonClick,
-            checked: isRecording,
-            ref: (el) => {
-              this._recordButton = el;
-            },
-            locked: isLocked
+            onClick: onRecordButtonClick
           }),
           button({
             id: "import-button",
             className: "devtools-button",
             title: L10N.getStr("recordings.import.tooltip"),
             onClick: onImportButtonClick
           })
         )
--- a/devtools/client/performance/test/browser_perf-button-states.js
+++ b/devtools/client/performance/test/browser_perf-button-states.js
@@ -11,48 +11,40 @@ const { initPerformanceInNewTab, teardow
 const { once } = require("devtools/client/performance/test/helpers/event-utils");
 
 add_task(function* () {
   let { panel } = yield initPerformanceInNewTab({
     url: SIMPLE_URL,
     win: window
   });
 
-  let { $, EVENTS, PerformanceController, PerformanceView } = panel.panelWin;
+  let { $, $$, EVENTS, PerformanceController, PerformanceView } = panel.panelWin;
+
   let recordButton = $("#main-record-button");
 
-  ok(!recordButton.hasAttribute("checked"),
-    "The record button should not be checked yet.");
-  ok(!recordButton.hasAttribute("locked"),
-    "The record button should not be locked yet.");
+  checkRecordButtonsStates(false, false);
 
   let uiStartClick = once(PerformanceView, EVENTS.UI_START_RECORDING);
   let recordingStarted = once(PerformanceController, EVENTS.RECORDING_STATE_CHANGE, {
     expectedArgs: { "1": "recording-started" }
   });
   let backendStartReady = once(PerformanceController,
                                EVENTS.BACKEND_READY_AFTER_RECORDING_START);
   let uiStateRecording = once(PerformanceView, EVENTS.UI_STATE_CHANGED, {
     expectedArgs: { "1": "recording" }
   });
 
   click(recordButton);
   yield uiStartClick;
 
-  ok(recordButton.hasAttribute("checked"),
-    "The record button should now be checked.");
-  ok(recordButton.hasAttribute("locked"),
-    "The record button should be locked.");
+  checkRecordButtonsStates(true, true);
 
   yield recordingStarted;
 
-  ok(recordButton.hasAttribute("checked"),
-    "The record button should still be checked.");
-  ok(!recordButton.hasAttribute("locked"),
-    "The record button should not be locked.");
+  checkRecordButtonsStates(true, false);
 
   yield backendStartReady;
   yield uiStateRecording;
 
   let uiStopClick = once(PerformanceView, EVENTS.UI_STOP_RECORDING);
   let recordingStopped = once(PerformanceController, EVENTS.RECORDING_STATE_CHANGE, {
     expectedArgs: { "1": "recording-stopped" }
   });
@@ -61,18 +53,24 @@ add_task(function* () {
   let uiStateRecorded = once(PerformanceView, EVENTS.UI_STATE_CHANGED, {
     expectedArgs: { "1": "recorded" }
   });
 
   click(recordButton);
   yield uiStopClick;
   yield recordingStopped;
 
-  ok(!recordButton.hasAttribute("checked"),
-    "The record button should not be checked.");
-  ok(!recordButton.hasAttribute("locked"),
-    "The record button should not be locked.");
+  checkRecordButtonsStates(false, false);
 
   yield backendStopReady;
   yield uiStateRecorded;
 
   yield teardownToolboxAndRemoveTab(panel);
+
+  function checkRecordButtonsStates(checked, locked) {
+    for (let button of $$(".record-button")) {
+      is(button.classList.contains("checked"), checked,
+         "The record button checked state should be " + checked);
+      is(button.disabled, locked,
+         "The record button locked state should be " + locked);
+    }
+  }
 });
--- a/devtools/client/performance/test/browser_perf-recording-selected-03.js
+++ b/devtools/client/performance/test/browser_perf-recording-selected-03.js
@@ -27,17 +27,17 @@ add_task(function* () {
   yield startRecording(panel);
 
   info("Selecting recording #0 and waiting for it to be displayed.");
 
   let selected = once(PerformanceController, EVENTS.RECORDING_SELECTED);
   RecordingsView.selectedIndex = 0;
   yield selected;
 
-  ok($("#main-record-button").hasAttribute("checked"),
+  ok($("#main-record-button").classList.contains("checked"),
     "Button is still checked after selecting another item.");
-  ok(!$("#main-record-button").hasAttribute("locked"),
+  ok(!$("#main-record-button").hasAttribute("disabled"),
     "Button is not locked after selecting another item.");
 
   yield stopRecording(panel);
 
   yield teardownToolboxAndRemoveTab(panel);
 });
--- a/devtools/client/themes/common.css
+++ b/devtools/client/themes/common.css
@@ -362,27 +362,29 @@ checkbox:-moz-focusring {
 
 /* Icon-only buttons */
 .devtools-button:empty::before,
 .devtools-toolbarbutton:not([label]):not([disabled]) > image {
   opacity: 0.8;
 }
 
 .devtools-button:hover:empty::before,
+.devtools-button.checked:empty::before,
 .devtools-button[checked]:empty::before,
 .devtools-button[open]:empty::before,
 .devtools-toolbarbutton:not([label]):hover > image,
 .devtools-toolbarbutton:not([label])[checked=true] > image,
 .devtools-toolbarbutton:not([label])[open=true] > image {
   opacity: 1;
 }
 
 .devtools-button:disabled,
 .devtools-button[disabled],
 .devtools-toolbarbutton[disabled] {
+  pointer-events: none;
   opacity: 0.5 !important;
 }
 
 .devtools-button[checked]:empty::before,
 .devtools-button[open]:empty::before,
 .devtools-button.checked::before,
 .devtools-toolbarbutton:not([label])[checked=true] > image,
 .devtools-toolbarbutton:not([label])[open=true] > image {
@@ -392,39 +394,41 @@ checkbox:-moz-focusring {
 /* Icon-and-text buttons */
 .devtools-toolbarbutton.icon-and-text .toolbarbutton-text {
   margin-inline-start: .5em !important;
   font-weight: 600;
 }
 
 /* Text-only buttons */
 .theme-light .devtools-toolbarbutton[label]:not([text-as-image]):not([type=menu-button]),
-.theme-light .devtools-toolbarbutton[data-text-only] {
+.theme-light .devtools-toolbarbutton[data-text-only],
+.theme-light .devtools-button:not(:empty) {
   background-color: var(--toolbar-tab-hover);
 }
 .theme-dark .devtools-toolbarbutton[label]:not([text-as-image]):not([type=menu-button]),
-.theme-dark .devtools-toolbarbutton[data-text-only] {
+.theme-dark .devtools-toolbarbutton[data-text-only],
+.theme-dark .devtools-button:not(:empty) {
   background-color: rgba(0, 0, 0, .2); /* Splitter */
 }
 
 /* Text-only button states */
-.theme-dark .devtools-button:not(:empty):not([disabled]):hover,
+.theme-dark .devtools-button:not(:empty):not(:disabled):hover,
 .theme-dark .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover {
   background: rgba(0, 0, 0, .3); /* Splitters */
 }
-.theme-light .devtools-button:not(:empty):not([disabled]):hover,
+.theme-light .devtools-button:not(:empty):not(:disabled):hover,
 .theme-light .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover {
   background: rgba(170, 170, 170, .3); /* Splitters */
 }
 
-.theme-dark .devtools-button:not(:empty):not([disabled]):hover:active,
+.theme-dark .devtools-button:not(:empty):not(:disabled):hover:active,
 .theme-dark .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover:active {
   background: rgba(0, 0, 0, .4); /* Splitters */
 }
-.theme-light .devtools-button:not(:empty):not([disabled]):hover:active,
+.theme-light .devtools-button:not(:empty):not(:disabled):hover:active,
 .theme-light .devtools-toolbarbutton:not(:-moz-any([checked=true],[disabled],[text-as-image]))[label]:hover:active {
   background: var(--toolbar-tab-hover-active);
 }
 
 .theme-dark .devtools-toolbarbutton:not([disabled])[label][checked=true],
 .theme-dark .devtools-toolbarbutton:not([disabled])[label][open],
 .theme-dark .devtools-button:not(:empty)[checked=true] {
   background: var(--theme-selection-background-semitransparent);
--- a/devtools/client/themes/performance.css
+++ b/devtools/client/themes/performance.css
@@ -111,27 +111,22 @@
 #main-record-button .button-text, #import-button .button-text {
   display: none;
 }
 
 .notice-container .record-button {
   padding: 5px !important;
 }
 
-.notice-container .record-button[checked],
-.notice-container .record-button[checked] {
+.notice-container .record-button.checked,
+.notice-container .record-button.checked {
   color: var(--theme-selection-color) !important;
   background: var(--theme-selection-background) !important;
 }
 
-.record-button[locked] {
-  pointer-events: none;
-  opacity: 0.5;
-}
-
 /* Sidebar & recording items */
 
 #recordings-pane {
   border-inline-end: 1px solid var(--theme-splitter-color);
   width: var(--sidebar-width);
 }
 
 #recording-controls-mount {