Devtools: [ElementsPanel] Add dom listeners in sidebars (issue 1104163003 by sergeyv@chromium.org)

0 views
Skip to first unread message

ser...@chromium.org

unread,
Apr 27, 2015, 1:01:50 PM4/27/15
to pfel...@chromium.org, lush...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, blink-rev...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
Reviewers: pfeldman, lushnikov,

Description:
Devtools: [ElementsPanel] Add dom listeners in sidebars

BUG=

Please review this at https://codereview.chromium.org/1104163003/

Base URL:
https://chromium.googlesource.com/chromium/blink.git@move-force-state

Affected files (+21, -4 lines):
M Source/devtools/front_end/elements/ElementsPanel.js
M Source/devtools/front_end/elements/MetricsSidebarPane.js
M Source/devtools/front_end/elements/PlatformFontsSidebarPane.js
M Source/devtools/front_end/elements/StylesSidebarPane.js


Index: Source/devtools/front_end/elements/ElementsPanel.js
diff --git a/Source/devtools/front_end/elements/ElementsPanel.js
b/Source/devtools/front_end/elements/ElementsPanel.js
index
eb95d4443df114fc85204cf4e9fa61601a2bdbd6..6079b9d5bfecafeff05a39095d3028546e30ec6f
100644
--- a/Source/devtools/front_end/elements/ElementsPanel.js
+++ b/Source/devtools/front_end/elements/ElementsPanel.js
@@ -675,12 +675,11 @@ WebInspector.ElementsPanel.prototype = {
{
/**
* @param {!WebInspector.ElementsTreeOutline} treeOutline
- * @this {WebInspector.ElementsPanel}
*/
function handleUndoRedo(treeOutline)
{
if (WebInspector.KeyboardShortcut.eventHasCtrlOrMeta(event)
&& !event.shiftKey && event.keyIdentifier === "U+005A") { // Z key
-
treeOutline.domModel().undo(this._updateSidebars.bind(this));
+ treeOutline.domModel().undo();
event.handled = true;
return;
}
@@ -688,7 +687,7 @@ WebInspector.ElementsPanel.prototype = {
var isRedoKey = WebInspector.isMac() ? event.metaKey &&
event.shiftKey && event.keyIdentifier === "U+005A" : // Z key
event.ctrlKey &&
event.keyIdentifier === "U+0059"; // Y key
if (isRedoKey) {
-
treeOutline.domModel().redo(this._updateSidebars.bind(this));
+ treeOutline.domModel().redo();
event.handled = true;
}
}
@@ -705,7 +704,7 @@ WebInspector.ElementsPanel.prototype = {
return;

if (!treeOutline.editing()) {
- handleUndoRedo.call(this, treeOutline);
+ handleUndoRedo.call(null, treeOutline);
if (event.handled)
return;
}
Index: Source/devtools/front_end/elements/MetricsSidebarPane.js
diff --git a/Source/devtools/front_end/elements/MetricsSidebarPane.js
b/Source/devtools/front_end/elements/MetricsSidebarPane.js
index
444c8c94e3066a75183568caba32e059e78ec119..fbc7b33ca4edcd22f5e9a277cd08bd714b379b66
100644
--- a/Source/devtools/front_end/elements/MetricsSidebarPane.js
+++ b/Source/devtools/front_end/elements/MetricsSidebarPane.js
@@ -60,6 +60,9 @@ WebInspector.MetricsSidebarPane.prototype = {

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.StyleSheetChanged,
this.update, this);

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.MediaQueryResultChanged,
this.update, this);

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.PseudoStateForced,
this.update, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.NodeInserted,
this.update, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.NodeRemoved,
this.update, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.DistributedNodesChanged,
this.update, this);

this._domModel.removeEventListener(WebInspector.DOMModel.Events.AttrModified,
this._attributesUpdated, this);

this._domModel.removeEventListener(WebInspector.DOMModel.Events.AttrRemoved,
this._attributesUpdated, this);

this._target.resourceTreeModel.removeEventListener(WebInspector.ResourceTreeModel.EventTypes.FrameResized,
this.update, this);
@@ -73,6 +76,9 @@ WebInspector.MetricsSidebarPane.prototype = {

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.StyleSheetChanged,
this.update, this);

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.MediaQueryResultChanged,
this.update, this);

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.PseudoStateForced,
this.update, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.NodeInserted,
this.update, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.NodeRemoved,
this.update, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.DistributedNodesChanged,
this.update, this);

this._domModel.addEventListener(WebInspector.DOMModel.Events.AttrModified,
this._attributesUpdated, this);

this._domModel.addEventListener(WebInspector.DOMModel.Events.AttrRemoved,
this._attributesUpdated, this);

this._target.resourceTreeModel.addEventListener(WebInspector.ResourceTreeModel.EventTypes.FrameResized,
this.update, this);
Index: Source/devtools/front_end/elements/PlatformFontsSidebarPane.js
diff --git a/Source/devtools/front_end/elements/PlatformFontsSidebarPane.js
b/Source/devtools/front_end/elements/PlatformFontsSidebarPane.js
index
faa8e1e10d3699745752a0701d69d5d9e4d36b0f..a2359bc1ba8afe01b972dcc7d7f94f7c9c177667
100644
--- a/Source/devtools/front_end/elements/PlatformFontsSidebarPane.js
+++ b/Source/devtools/front_end/elements/PlatformFontsSidebarPane.js
@@ -68,6 +68,9 @@ WebInspector.PlatformFontsSidebarPane.prototype = {

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.StyleSheetChanged,
this.update, this);

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.MediaQueryResultChanged,
this.update, this);

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.PseudoStateForced,
this.update, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.NodeInserted,
this.update, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.NodeRemoved,
this.update, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.DistributedNodesChanged,
this.update, this);

this._domModel.removeEventListener(WebInspector.DOMModel.Events.AttrModified,
this.update, this);

this._domModel.removeEventListener(WebInspector.DOMModel.Events.AttrRemoved,
this.update, this);

this._domModel.removeEventListener(WebInspector.DOMModel.Events.CharacterDataModified,
this.update, this);
@@ -81,6 +84,9 @@ WebInspector.PlatformFontsSidebarPane.prototype = {

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.StyleSheetChanged,
this.update, this);

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.MediaQueryResultChanged,
this.update, this);

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.PseudoStateForced,
this.update, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.NodeInserted,
this.update, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.NodeRemoved,
this.update, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.DistributedNodesChanged,
this.update, this);

this._domModel.addEventListener(WebInspector.DOMModel.Events.AttrModified,
this.update, this);

this._domModel.addEventListener(WebInspector.DOMModel.Events.AttrRemoved,
this.update, this);

this._domModel.addEventListener(WebInspector.DOMModel.Events.CharacterDataModified,
this.update, this);
Index: Source/devtools/front_end/elements/StylesSidebarPane.js
diff --git a/Source/devtools/front_end/elements/StylesSidebarPane.js
b/Source/devtools/front_end/elements/StylesSidebarPane.js
index
3355ad8238dac9cc8a305484b1f6a7dfd2484a9f..e39caef4d67c6ffcc9925b5621d586ad1c89ee19
100644
--- a/Source/devtools/front_end/elements/StylesSidebarPane.js
+++ b/Source/devtools/front_end/elements/StylesSidebarPane.js
@@ -377,6 +377,9 @@ WebInspector.StylesSidebarPane.prototype = {

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.StyleSheetChanged,
this._styleSheetOrMediaQueryResultChanged, this);

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.MediaQueryResultChanged,
this._styleSheetOrMediaQueryResultChanged, this);

this._cssModel.removeEventListener(WebInspector.CSSStyleModel.Events.PseudoStateForced,
this._styleSheetOrMediaQueryResultChanged, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.NodeInserted,
this._styleSheetOrMediaQueryResultChanged, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.NodeRemoved,
this._styleSheetOrMediaQueryResultChanged, this);
+
this._domModel.removeEventListener(WebInspector.DOMModel.Events.DistributedNodesChanged,
this._styleSheetOrMediaQueryResultChanged, this);

this._domModel.removeEventListener(WebInspector.DOMModel.Events.AttrModified,
this._attributeChanged, this);

this._domModel.removeEventListener(WebInspector.DOMModel.Events.AttrRemoved,
this._attributeChanged, this);

this._target.resourceTreeModel.removeEventListener(WebInspector.ResourceTreeModel.EventTypes.FrameResized,
this._frameResized, this);
@@ -390,6 +393,9 @@ WebInspector.StylesSidebarPane.prototype = {

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.StyleSheetChanged,
this._styleSheetOrMediaQueryResultChanged, this);

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.MediaQueryResultChanged,
this._styleSheetOrMediaQueryResultChanged, this);

this._cssModel.addEventListener(WebInspector.CSSStyleModel.Events.PseudoStateForced,
this._styleSheetOrMediaQueryResultChanged, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.NodeInserted,
this._styleSheetOrMediaQueryResultChanged, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.NodeRemoved,
this._styleSheetOrMediaQueryResultChanged, this);
+
this._domModel.addEventListener(WebInspector.DOMModel.Events.DistributedNodesChanged,
this._styleSheetOrMediaQueryResultChanged, this);

this._domModel.addEventListener(WebInspector.DOMModel.Events.AttrModified,
this._attributeChanged, this);

this._domModel.addEventListener(WebInspector.DOMModel.Events.AttrRemoved,
this._attributeChanged, this);

this._target.resourceTreeModel.addEventListener(WebInspector.ResourceTreeModel.EventTypes.FrameResized,
this._frameResized, this);


pfel...@chromium.org

unread,
Apr 28, 2015, 1:53:21 AM4/28/15
to ser...@chromium.org, lush...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, blink-rev...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1104163003/diff/1/Source/devtools/front_end/elements/MetricsSidebarPane.js
File Source/devtools/front_end/elements/MetricsSidebarPane.js (right):

https://codereview.chromium.org/1104163003/diff/1/Source/devtools/front_end/elements/MetricsSidebarPane.js#newcode63
Source/devtools/front_end/elements/MetricsSidebarPane.js:63:
this._domModel.removeEventListener(WebInspector.DOMModel.Events.NodeInserted,
this.update, this);
Can we extract a helper object that would listen to these and would fire
a single event in return?
Alternatively, we could introduce a new artificial event that would fire
upon _any_ of these DOM changes.

https://codereview.chromium.org/1104163003/

ser...@chromium.org

unread,
Apr 28, 2015, 4:53:26 AM4/28/15
to pfel...@chromium.org, lush...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, blink-rev...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1104163003/diff/1/Source/devtools/front_end/elements/MetricsSidebarPane.js
File Source/devtools/front_end/elements/MetricsSidebarPane.js (right):

https://codereview.chromium.org/1104163003/diff/1/Source/devtools/front_end/elements/MetricsSidebarPane.js#newcode63
Source/devtools/front_end/elements/MetricsSidebarPane.js:63:
this._domModel.removeEventListener(WebInspector.DOMModel.Events.NodeInserted,
this.update, this);
On 2015/04/28 05:53:20, pfeldman wrote:
> Can we extract a helper object that would listen to these and would
fire a
> single event in return?
> Alternatively, we could introduce a new artificial event that would
fire upon
> _any_ of these DOM changes.

Done.

https://codereview.chromium.org/1104163003/

pfel...@chromium.org

unread,
Apr 29, 2015, 9:53:16 AM4/29/15
to ser...@chromium.org, lush...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, blink-rev...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1104163003/diff/40001/Source/devtools/front_end/sdk/DOMModel.js
File Source/devtools/front_end/sdk/DOMModel.js (right):

https://codereview.chromium.org/1104163003/diff/40001/Source/devtools/front_end/sdk/DOMModel.js#newcode1132
Source/devtools/front_end/sdk/DOMModel.js:1132: this._lastMutationId =
(this._lastMutationId || 0) + 1;
fast return if no listeners.

https://codereview.chromium.org/1104163003/diff/40001/Source/devtools/front_end/sdk/DOMModel.js#newcode1144
Source/devtools/front_end/sdk/DOMModel.js:1144: for (var observerInfo of
this._mutationsObservers)
this.dispatchEventToListeners(DOMMutated);

https://codereview.chromium.org/1104163003/diff/40001/Source/devtools/front_end/sdk/DOMModel.js#newcode1173
Source/devtools/front_end/sdk/DOMModel.js:1173: }
delete if empty (along with the methods).

https://codereview.chromium.org/1104163003/

ser...@chromium.org

unread,
Apr 30, 2015, 8:34:13 AM4/30/15
to pfel...@chromium.org, lush...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, blink-rev...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1104163003/diff/40001/Source/devtools/front_end/sdk/DOMModel.js
File Source/devtools/front_end/sdk/DOMModel.js (right):

https://codereview.chromium.org/1104163003/diff/40001/Source/devtools/front_end/sdk/DOMModel.js#newcode1132
Source/devtools/front_end/sdk/DOMModel.js:1132: this._lastMutationId =
(this._lastMutationId || 0) + 1;
On 2015/04/29 13:53:15, pfeldman wrote:
> fast return if no listeners.

Done.

https://codereview.chromium.org/1104163003/diff/40001/Source/devtools/front_end/sdk/DOMModel.js#newcode1144
Source/devtools/front_end/sdk/DOMModel.js:1144: for (var observerInfo of
this._mutationsObservers)
On 2015/04/29 13:53:15, pfeldman wrote:
> this.dispatchEventToListeners(DOMMutated);

Done.

https://codereview.chromium.org/1104163003/diff/40001/Source/devtools/front_end/sdk/DOMModel.js#newcode1173
Source/devtools/front_end/sdk/DOMModel.js:1173: }
On 2015/04/29 13:53:15, pfeldman wrote:
> delete if empty (along with the methods).

Done.

https://codereview.chromium.org/1104163003/

ser...@chromium.org

unread,
May 5, 2015, 5:23:40 AM5/5/15
to pfel...@chromium.org, lush...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, blink-rev...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
ptal

https://codereview.chromium.org/1104163003/

pfel...@chromium.org

unread,
May 5, 2015, 10:13:07 AM5/5/15
to ser...@chromium.org, lush...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, blink-rev...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1104163003/diff/80001/Source/devtools/front_end/elements/MetricsSidebarPane.js
File Source/devtools/front_end/elements/MetricsSidebarPane.js (right):

https://codereview.chromium.org/1104163003/diff/80001/Source/devtools/front_end/elements/MetricsSidebarPane.js#newcode78
Source/devtools/front_end/elements/MetricsSidebarPane.js:78:
this._domModel.addEventListener(WebInspector.DOMModel.Events.AttrModified,
this._attributesUpdated, this);
I assume these should be removed now. Also, you probably want the same
for CSS?

https://codereview.chromium.org/1104163003/diff/80001/Source/devtools/front_end/sdk/DOMModel.js
File Source/devtools/front_end/sdk/DOMModel.js (right):

https://codereview.chromium.org/1104163003/diff/80001/Source/devtools/front_end/sdk/DOMModel.js#newcode1131
Source/devtools/front_end/sdk/DOMModel.js:1131: _fireMutationEvent:
function()
_scheduleMutationEvent

https://codereview.chromium.org/1104163003/

ser...@chromium.org

unread,
May 6, 2015, 4:35:54 AM5/6/15
to pfel...@chromium.org, lush...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, blink-rev...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1104163003/diff/80001/Source/devtools/front_end/elements/MetricsSidebarPane.js
File Source/devtools/front_end/elements/MetricsSidebarPane.js (right):

https://codereview.chromium.org/1104163003/diff/80001/Source/devtools/front_end/elements/MetricsSidebarPane.js#newcode78
Source/devtools/front_end/elements/MetricsSidebarPane.js:78:
this._domModel.addEventListener(WebInspector.DOMModel.Events.AttrModified,
this._attributesUpdated, this);
On 2015/05/05 14:13:07, pfeldman_ooo_may11 wrote:
> I assume these should be removed now. Also, you probably want the same
for CSS?

AttrModified/AttrRemoved usually have separate handler, like here:
_attributeChanged.

I can do the same for css, but in a separate patch

https://codereview.chromium.org/1104163003/diff/80001/Source/devtools/front_end/sdk/DOMModel.js
File Source/devtools/front_end/sdk/DOMModel.js (right):

https://codereview.chromium.org/1104163003/diff/80001/Source/devtools/front_end/sdk/DOMModel.js#newcode1131
Source/devtools/front_end/sdk/DOMModel.js:1131: _fireMutationEvent:
function()
On 2015/05/05 14:13:07, pfeldman_ooo_may11 wrote:
> _scheduleMutationEvent

Done.

https://codereview.chromium.org/1104163003/
Reply all
Reply to author
Forward
0 new messages