[webkit-reviews] review denied: [Bug 108073] Web Inspector: Allow user to change dock side by dragging toolbar : [Attachment 185465] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Dmitry Gozman
<dgozman at chromium.org>'s request for review:
Bug 108073: Web Inspector: Allow user to change dock side by dragging toolbar
https://bugs.webkit.org/show_bug.cgi?id=108073

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

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


> Source/WebCore/inspector/front-end/DockController.js:198
> +    changeDockSide: function(side) {

{ should go to the next line.

> Source/WebCore/inspector/front-end/Toolbar.js:106
> +	   return !!(WebInspector.dockController &&
WebInspector.dockController.dockSide() ==
WebInspector.DockController.State.DockedToBottom);

Should have been !!WebInspector.dockController &&
WebInspector.dockController.isDockedToBottom();

> Source/WebCore/inspector/front-end/Toolbar.js:-144
> -	       InspectorFrontendHost.setAttachedWindowHeight(height);

Mac port will get upset about regressing this. Especially given they don't
support dock-to-right. You should maintain existing behavior for the ports that
have Preferences.showDockToRight === false.

> Source/WebCore/inspector/front-end/UIUtils.js:100
> +WebInspector.cancelDragEvents = function(element)

You should instead keep this private and allow elementDrag handler terminate
current drag operation. For example, it could return "true" to terminate the
drag. Otherwise this API becomes error prone