[webkit-reviews] review granted: [Bug 138941] Web Inspector: create canvas content view and details sidebar panel : [Attachment 313351] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 19 19:34:14 PDT 2017


Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 138941: Web Inspector: create canvas content view and details sidebar panel
https://bugs.webkit.org/show_bug.cgi?id=138941

Attachment 313351: Patch

https://bugs.webkit.org/attachment.cgi?id=313351&action=review




--- Comment #17 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 313351
  --> https://bugs.webkit.org/attachment.cgi?id=313351
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=313351&action=review

r=me

> Source/WebInspectorUI/UserInterface/Images/Canvas.svg:2
> +<?xml version="1.0" encoding="utf-8"?>
> +<!-- Copyright © 2017 Apple Inc. All rights reserved. -->

File a bug on GTK to add an Image.

> Source/WebInspectorUI/UserInterface/Models/Canvas.js:143
> +	   if (this._domNode)
> +	       cookie[WebInspector.Canvas.NodePathCookieKey] =
this._domNode.path;

Should this be an `else` of CSSCanvasName? Because that DOMNode won't have a
path.

> Source/WebInspectorUI/UserInterface/Views/CanvasDetailsSidebarPanel.js:65
> +	   if (this._node) {
> +	      
this._node.removeEventListener(WebInspector.DOMNode.Event.AttributeModified,
this._refreshSourceSection, this);
> +	      
this._node.removeEventListener(WebInspector.DOMNode.Event.AttributeRemoved,
this._refreshSourceSection, this);
> +	   }

Probably want to assign this._node = null after removing event listeners.


More information about the webkit-reviews mailing list