Bug 1173049 - Markers on the edge of time range are hard to interact with, r=jsantell
authorVictor Porof <vporof@mozilla.com>
Tue, 04 Aug 2015 12:18:58 +0200
changeset 287714 3f94e478ad8eb5590547f594f43f8d7fdde14fe6
parent 287713 991af90f693d1554b319ee1283e35097c2d6e740
child 287715 102bc51e9a574541c4a1589ac2c4e6bdcdb7a082
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjsantell
bugs1173049
milestone42.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 1173049 - Markers on the edge of time range are hard to interact with, r=jsantell
browser/devtools/performance/modules/widgets/marker-details.js
browser/devtools/performance/modules/widgets/marker-view.js
browser/devtools/performance/test/browser_timeline-waterfall-background.js
browser/devtools/performance/test/browser_timeline-waterfall-generic.js
--- a/browser/devtools/performance/modules/widgets/marker-details.js
+++ b/browser/devtools/performance/modules/widgets/marker-details.js
@@ -31,33 +31,54 @@ function MarkerDetails(parent, splitter)
   this._parent = parent;
   this._splitter = splitter;
 
   this._onClick = this._onClick.bind(this);
   this._onSplitterMouseUp = this._onSplitterMouseUp.bind(this);
 
   this._parent.addEventListener("click", this._onClick);
   this._splitter.addEventListener("mouseup", this._onSplitterMouseUp);
+
+  this.hidden = true;
 }
 
 MarkerDetails.prototype = {
   /**
    * Sets this view's width.
-   * @param boolean
+   * @param number
    */
   set width(value) {
     this._parent.setAttribute("width", value);
   },
 
   /**
+   * Sets this view's width.
+   * @return number
+   */
+  get width() {
+    return +this._parent.getAttribute("width");
+  },
+
+  /**
    * Sets this view's visibility.
    * @param boolean
    */
   set hidden(value) {
-    this._parent.hidden = value;
+    if (this._parent.hidden != value) {
+      this._parent.hidden = value;
+      this.emit("resize");
+    }
+  },
+
+  /**
+   * Gets this view's visibility.
+   * @param boolean
+   */
+  get hidden() {
+    return this._parent.hidden;
   },
 
   /**
    * Clears the marker details from this view.
    */
   empty: function() {
     this._parent.innerHTML = "";
   },
--- a/browser/devtools/performance/modules/widgets/marker-view.js
+++ b/browser/devtools/performance/modules/widgets/marker-view.js
@@ -6,24 +6,25 @@
 /**
  * This file contains the "marker" view, essentially a detailed list
  * of all the markers in the timeline data.
  */
 
 const { Cc, Ci, Cu, Cr } = require("chrome");
 const { Heritage } = require("resource:///modules/devtools/ViewHelpers.jsm");
 const { AbstractTreeItem } = require("resource:///modules/devtools/AbstractTreeItem.jsm");
+
 loader.lazyRequireGetter(this, "MarkerUtils",
   "devtools/performance/marker-utils");
 
-
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 
 const LEVEL_INDENT = 10; // px
 const ARROW_NODE_OFFSET = -15; // px
+const WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS = 20; // px
 const WATERFALL_MARKER_SIDEBAR_WIDTH = 175; // px
 const WATERFALL_MARKER_TIMEBAR_WIDTH_MIN = 5; // px
 
 /**
  * A detailed waterfall view for the timeline data.
  *
  * @param MarkerView owner
  *        The MarkerView considered the "owner" marker. This newly created
@@ -50,17 +51,19 @@ function MarkerView({ owner, marker, lev
 }
 
 MarkerView.prototype = Heritage.extend(AbstractTreeItem.prototype, {
   /**
    * Calculates and stores the available width for the waterfall.
    * This should be invoked every time the container node is resized.
    */
   recalculateBounds: function() {
-    this.root._waterfallWidth = this.bounds.width - WATERFALL_MARKER_SIDEBAR_WIDTH;
+    this.root._waterfallWidth = this.bounds.width
+      - WATERFALL_MARKER_SIDEBAR_WIDTH
+      - WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS;
   },
 
   /**
    * Sets a list of marker types to be filtered out of this view.
    * @param Array<String> filter
    */
   set filter(filter) {
     this.root._filter = filter;
@@ -284,9 +287,10 @@ function isMarkerInRange(e, start, end) 
 
   return (m_start >= start && m_end <= end) || // bounds inside
          (m_start < start && m_end > end) || // bounds outside
          (m_start < start && m_end >= start && m_end <= end) || // overlap start
          (m_end > end && m_start >= start && m_start <= end); // overlap end
 }
 
 exports.MarkerView = MarkerView;
+exports.WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS = WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS;
 exports.WATERFALL_MARKER_SIDEBAR_WIDTH = WATERFALL_MARKER_SIDEBAR_WIDTH;
--- a/browser/devtools/performance/test/browser_timeline-waterfall-background.js
+++ b/browser/devtools/performance/test/browser_timeline-waterfall-background.js
@@ -4,16 +4,17 @@
 /**
  * Tests if the waterfall background is a 1px high canvas stretching across
  * the container bounds.
  */
 
 function* spawnTest() {
   let { target, panel } = yield initPerformance(SIMPLE_URL);
   let { $, EVENTS, PerformanceController, OverviewView, DetailsView, WaterfallView } = panel.panelWin;
+  let { WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS } = devtools.require("devtools/performance/marker-view");
 
   yield startRecording(panel);
   ok(true, "Recording has started.");
 
   let updated = 0;
   OverviewView.on(EVENTS.OVERVIEW_RENDERED, () => updated++);
 
   ok((yield waitUntil(() => updated > 0)),
@@ -33,17 +34,18 @@ function* spawnTest() {
   ok(true, "Recording has rendered.");
 
   // Test the waterfall background.
 
   let parentWidth = $("#waterfall-view").getBoundingClientRect().width;
   let sidebarWidth = $(".waterfall-sidebar").getBoundingClientRect().width;
   let detailsWidth = $("#waterfall-details").getBoundingClientRect().width;
   let waterfallWidth = WaterfallView._markersRoot._waterfallWidth;
-  is(waterfallWidth, parentWidth - sidebarWidth - detailsWidth,
+
+  is(waterfallWidth, parentWidth - sidebarWidth - detailsWidth - WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS,
     "The waterfall width is correct.")
 
   ok(WaterfallView._waterfallHeader._canvas,
     "A canvas should be created after the recording ended.");
   ok(WaterfallView._waterfallHeader._ctx,
     "A 2d context should be created after the recording ended.");
 
   is(WaterfallView._waterfallHeader._canvas.width, waterfallWidth,
--- a/browser/devtools/performance/test/browser_timeline-waterfall-generic.js
+++ b/browser/devtools/performance/test/browser_timeline-waterfall-generic.js
@@ -2,32 +2,41 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
  * Tests if the waterfall is properly built after finishing a recording.
  */
 
 function* spawnTest() {
   let { target, panel } = yield initPerformance(SIMPLE_URL);
-  let { $, $$, EVENTS, PerformanceController, OverviewView, WaterfallView } = panel.panelWin;
+  let { $, $$, EVENTS, PerformanceController, OverviewView, WaterfallView, DetailsView } = panel.panelWin;
+  let { WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS } = devtools.require("devtools/performance/marker-view");
 
   yield startRecording(panel);
   ok(true, "Recording has started.");
 
   let updated = 0;
   OverviewView.on(EVENTS.OVERVIEW_RENDERED, () => updated++);
 
   ok((yield waitUntil(() => updated > 0)),
     "The overview graphs were updated a bunch of times.");
   ok((yield waitUntil(() => PerformanceController.getCurrentRecording().getMarkers().length > 0)),
     "There are some markers available.");
 
+  let rendered = Promise.all([
+    DetailsView.selectView("waterfall"),
+    once(WaterfallView, EVENTS.WATERFALL_RENDERED)
+  ]);
+
   yield stopRecording(panel);
   ok(true, "Recording has ended.");
 
+  yield rendered;
+  ok(true, "Recording has rendered.");
+
   // Test the header container.
 
   ok($(".waterfall-header-container"),
     "A header container should have been created.");
 
   // Test the header sidebar (left).
 
   ok($(".waterfall-header-container > .waterfall-sidebar"),
@@ -53,11 +62,48 @@ function* spawnTest() {
 
   // Test the markers waterfall (right).
 
   ok($$(".waterfall-tree-item > .waterfall-marker").length,
     "Some marker waterfall nodes should have been created.");
   ok($$(".waterfall-tree-item > .waterfall-marker > .waterfall-marker-bar").length,
     "Some marker color bars should have been created inside the waterfall.");
 
+  // Test the sidebar
+
+  let detailsView = WaterfallView.details;
+  let markersRoot = WaterfallView._markersRoot;
+
+  let parentWidthBefore = $("#waterfall-view").getBoundingClientRect().width;
+  let sidebarWidthBefore = $(".waterfall-sidebar").getBoundingClientRect().width;
+  let detailsWidthBefore = $("#waterfall-details").getBoundingClientRect().width;
+
+  ok(detailsView.hidden,
+    "The details view in the waterfall view is hidden by default.");
+  is(detailsWidthBefore, 0,
+    "The details view width should be 0 when hidden.");
+  is(markersRoot._waterfallWidth, parentWidthBefore - sidebarWidthBefore - WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS,
+    "The waterfall width is correct.")
+
+  let receivedFocusEvent = markersRoot.once("focus");
+  let waterfallRerendered = once(WaterfallView, EVENTS.WATERFALL_RENDERED);
+  WaterfallView._markersRoot.getChild(0).focus();
+  yield receivedFocusEvent;
+  yield waterfallRerendered;
+
+  let parentWidthAfter = $("#waterfall-view").getBoundingClientRect().width;
+  let sidebarWidthAfter = $(".waterfall-sidebar").getBoundingClientRect().width;
+  let detailsWidthAfter = $("#waterfall-details").getBoundingClientRect().width;
+
+  ok(!detailsView.hidden,
+    "The details view in the waterfall view is now visible.");
+  is(parentWidthBefore, parentWidthAfter,
+    "The parent view's width should not have changed.");
+  is(sidebarWidthBefore, sidebarWidthAfter,
+    "The sidebar view's width should not have changed.");
+  isnot(detailsWidthAfter, 0,
+    "The details view width should not be 0 when visible.");
+  is(markersRoot._waterfallWidth, parentWidthAfter - sidebarWidthAfter - detailsWidthAfter - WATERFALL_MARKER_SIDEBAR_SAFE_BOUNDS,
+    "The waterfall width is correct (2).")
+
   yield teardown(panel);
   finish();
 }