Attachment 361425 Details for Bug 194365 – Patch

[patch] Patch

bug-194365-20190207115732.patch (text/plain), 9.88 KB, created by Devin Rousso on 2019-02-07 11:57:33 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-02-07 11:57:33 PST

Size: 9.88 KB

patch

obsolete

>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 278ff91da89f323a5e789dbbc13ce546cb96b8cf..5edbb3e6475ba30ec8ef55009f0ad1f47b3cef37 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,35 @@
>+2019-02-07  Devin Rousso  <drousso@apple.com>
>+
>+        Web Inspector: Timelines: clicking on an empty space in the overview should deselect any selected record bar
>+        https://bugs.webkit.org/show_bug.cgi?id=194365
>+        <rdar://problem/47868426>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UserInterface/Views/TimelineRecordBar.js:
>+        (WI.TimelineRecordBar.prototype._handleClick):
>+        Mark the "click" event so that later listeners know it was handled by `WI.TimelineRecordBar`.
>+
>+        * UserInterface/Views/TimelineOverview.js:
>+        (WI.TimelineOverview):
>+        (WI.TimelineOverview.prototype._instrumentAdded):
>+        (WI.TimelineOverview.prototype._instrumentRemoved):
>+        (WI.TimelineOverview.prototype._handleGraphsContainerClick): Added.
>+        (WI.TimelineOverview.prototype._handleOverviewGraphRecordSelected): Added.
>+        (WI.TimelineOverview.prototype._recordSelected):
>+        Listen for "click" on the graph container and deselect all records when fired, unless the
>+        click was marked by a `WI.TimelineRecordBar`.
>+
>+        * UserInterface/Views/TimelineRecordingContentView.js:
>+        (WI.TimelineRecordingContentView.prototype._recordSelected):
>+        Ensure that all `WI.TimelineView` update their selected record whenever it changes for any
>+        other `WI.TimelineView` (or if there is no selected record).
>+
>+        * UserInterface/Views/TimelineOverviewGraph.js:
>+        (WI.TimelineOverviewGraph.prototype.didLayoutSubtree): Added.
>+        Drive-by: since `WI.TimelineRecordBar` are reused when combining, we need to re-determine
>+        which one holds the currently selected record.
>+
> 2019-02-06  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: DOM: don't send the entire function string with each event listener
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js b/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js
>index f685c54b94eb588502fe5eafed4a61157f666609..5c79c608e3723ff75690988e4008980d9363b457 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js
>@@ -53,6 +53,7 @@ WI.TimelineOverview = class TimelineOverview extends WI.View
> 
>         this._graphsContainerView = new WI.View;
>         this._graphsContainerView.element.classList.add("graphs-container");
>+        this._graphsContainerView.element.addEventListener("click", this._handleGraphsContainerClick.bind(this));
>         this.addSubview(this._graphsContainerView);
> 
>         this._selectedTimelineRecord = null;
>@@ -636,7 +637,7 @@ WI.TimelineOverview = class TimelineOverview extends WI.View
>         this._treeElementsByTypeMap.set(timeline.type, treeElement);
> 
>         let overviewGraph = WI.TimelineOverviewGraph.createForTimeline(timeline, this);
>-        overviewGraph.addEventListener(WI.TimelineOverviewGraph.Event.RecordSelected, this._recordSelected, this);
>+        overviewGraph.addEventListener(WI.TimelineOverviewGraph.Event.RecordSelected, this._handleOverviewGraphRecordSelected, this);
>         this._overviewGraphsByTypeMap.set(timeline.type, overviewGraph);
>         this._graphsContainerView.insertSubviewBefore(overviewGraph, this._graphsContainerView.subviews[insertionIndex]);
> 
>@@ -662,7 +663,7 @@ WI.TimelineOverview = class TimelineOverview extends WI.View
>         let shouldSuppressSelectSibling = true;
>         this._timelinesTreeOutline.removeChild(treeElement, shouldSuppressOnDeselect, shouldSuppressSelectSibling);
> 
>-        overviewGraph.removeEventListener(WI.TimelineOverviewGraph.Event.RecordSelected, this._recordSelected, this);
>+        overviewGraph.removeEventListener(WI.TimelineOverviewGraph.Event.RecordSelected, this._handleOverviewGraphRecordSelected, this);
>         this._graphsContainerView.removeSubview(overviewGraph);
> 
>         this._overviewGraphsByTypeMap.delete(timeline.type);
>@@ -674,6 +675,15 @@ WI.TimelineOverview = class TimelineOverview extends WI.View
>         this._timelineRuler.addMarker(event.data.marker);
>     }
> 
>+    _handleGraphsContainerClick(event)
>+    {
>+        // Set when a WI.TimelineRecordBar receives the "click" first and is about to be selected.
>+        if (event.__timelineRecordBarClick)
>+            return;
>+
>+        this._recordSelected(null, null);
>+    }
>+
>     _timelineRulerMouseDown(event)
>     {
>         this._timelineRulerSelectionChanged = false;
>@@ -710,13 +720,23 @@ WI.TimelineOverview = class TimelineOverview extends WI.View
>         this.dispatchEventToListeners(WI.TimelineOverview.Event.TimeRangeSelectionChanged);
>     }
> 
>-    _recordSelected(event)
>+    _handleOverviewGraphRecordSelected(event)
>     {
>         let {record, recordBar} = event.data;
>-        if (!record || record === this._selectedTimelineRecord)
>+
>+        // Ignore deselection events, as they are handled by the newly selected record's timeline.
>+        if (!record)
>             return;
> 
>-        if (this._selectedTimelineRecord && this._selectedTimelineRecord.type !== record.type) {
>+        this._recordSelected(record, recordBar);
>+    }
>+
>+    _recordSelected(record, recordBar)
>+    {
>+        if (record === this._selectedTimelineRecord)
>+            return;
>+
>+        if (this._selectedTimelineRecord && (!record || this._selectedTimelineRecord.type !== record.type)) {
>             let timelineOverviewGraph = this._overviewGraphsByTypeMap.get(this._selectedTimelineRecord.type);
>             console.assert(timelineOverviewGraph);
>             if (timelineOverviewGraph)
>@@ -734,7 +754,7 @@ WI.TimelineOverview = class TimelineOverview extends WI.View
>             }
> 
>             let startTime = firstRecord instanceof WI.RenderingFrameTimelineRecord ? firstRecord.frameIndex : firstRecord.startTime;
>-            let endTime = lastRecord instanceof WI.RenderingFrameTimelineRecord ? lastRecord.frameIndex : lastRecord.startTime;
>+            let endTime = lastRecord instanceof WI.RenderingFrameTimelineRecord ? lastRecord.frameIndex : lastRecord.endTime;
> 
>             if (startTime < this.selectionStartTime || endTime > this.selectionStartTime + this.selectionDuration) {
>                 let selectionPadding = this.secondsPerPixel * 10;
>@@ -743,15 +763,7 @@ WI.TimelineOverview = class TimelineOverview extends WI.View
>             }
>         }
> 
>-        for (let [type, overviewGraph] of this._overviewGraphsByTypeMap) {
>-            if (overviewGraph !== event.target)
>-                continue;
>-
>-            let timeline = this._recording.timelines.get(type);
>-            console.assert(timeline, "Timeline recording missing timeline type", type);
>-            this.dispatchEventToListeners(WI.TimelineOverview.Event.RecordSelected, {timeline, record: this._selectedTimelineRecord});
>-            return;
>-        }
>+        this.dispatchEventToListeners(WI.TimelineOverview.Event.RecordSelected, {record: this._selectedTimelineRecord});
>     }
> 
>     _resetSelection()
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js b/Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js
>index 1e9f2f1a7e61ed4c27354535f9c9e37c3dff9d48..317353adbf0c01f581687f2d8e3c7a76ba1e88c2 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js
>@@ -253,6 +253,13 @@ WI.TimelineOverviewGraph = class TimelineOverviewGraph extends WI.View
>         super.needsLayout();
>     }
> 
>+    didLayoutSubtree()
>+    {
>+        super.didLayoutSubtree();
>+
>+        this.updateSelectedRecord();
>+    }
>+
>     // TimelineRecordBar delegate
> 
>     timelineRecordBarClicked(timelineRecordBar)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js b/Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js
>index f1885afe99d51ae3a978f31d89c6d6e0d00bd401..0dc2d6270c50ac8f0424065ea59c570c7f0d7bff 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js
>@@ -388,6 +388,9 @@ WI.TimelineRecordBar = class TimelineRecordBar extends WI.Object
> 
>     _handleClick(event)
>     {
>+        // Ensure that the container "click" listener added by `WI.TimelineOverview` isn't called.
>+        event.__timelineRecordBarClick = true;
>+
>         if (this._delegate.timelineRecordBarClicked)
>             this._delegate.timelineRecordBarClicked(this);
>     }
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js b/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js
>index a5615870f727223652952727775f206c0943a697..4c70505926bdbeb3d86675461367f597269b012e 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js
>@@ -697,13 +697,17 @@ WI.TimelineRecordingContentView = class TimelineRecordingContentView extends WI.
> 
>     _recordSelected(event)
>     {
>-        let {record, timeline} = event.data;
>-        let timelineView = this._timelineViewMap.get(timeline);
>+        let {record} = event.data;
> 
>-        if (record && timelineView !== this.currentTimelineView)
>-            this.showTimelineViewForTimeline(timeline);
>+        for (let timelineView of this._timelineViewMap.values()) {
>+            let recordMatchesTimeline = record && timelineView.representedObject.type === record.type;
> 
>-        timelineView.selectRecord(record);
>+            if (recordMatchesTimeline && timelineView !== this.currentTimelineView)
>+                this.showTimelineViewForTimeline(timelineView.representedObject);
>+
>+            if (!record || recordMatchesTimeline)
>+                timelineView.selectRecord(record);
>+        }
>     }
> 
>     _timelineSelected()

Actions: View | Formatted Diff | Diff

Attachments on bug 194365: 361425