[webkit-reviews] review denied: [Bug 66131] Web Inspector: maintain visible view hierarchy and dispatch common view events automatically : [Attachment 103917] patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 66131: Web Inspector: maintain visible view hierarchy and dispatch common
view events automatically
https://bugs.webkit.org/show_bug.cgi?id=66131

Attachment 103917: patch
https://bugs.webkit.org/attachment.cgi?id=103917&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103917&action=review


We need to make sure that the view with the attached element is in the view
hierarchy.

> Source/WebCore/ChangeLog:3
> +	   Web Inspector: maintain visible view hierarchy and dispatch common
view events automatically

Does this change fix any bugs?

> Source/WebCore/inspector/front-end/Drawer.js:65
> +	       this.removeChild(this._visibleView);

Why do we do this? We should either view.hide or detach / removeChild.

> Source/WebCore/inspector/front-end/NetworkPanel.js:1257
> +    this.addChild(this._networkLogView);

addChildView

> Source/WebCore/inspector/front-end/NetworkPanel.js:1390
>	       this.visibleView.detach();

detach should remove child from hierarchy.

> Source/WebCore/inspector/front-end/NetworkPanel.js:1407
> +	       this.removeChild(this.visibleView);

ditto

> Source/WebCore/inspector/front-end/Panel.js:390
> +	   this.dispatchToVisibleChildren("resize");

This should be on view, you should distinguish doResize and onResize.

> Source/WebCore/inspector/front-end/ResourcesPanel.js:519
> +	   this.addChild(view);

Again, hide and addChild should not be invoked next to each other.

> Source/WebCore/inspector/front-end/ResourcesPanel.js:534
> +	   this.removeChild(this.visibleView);

detach / removeChild together.

> Source/WebCore/inspector/front-end/ScriptsPanel.js:672
> +	   sourceFrame.detach();

ditto