Patch to follow.
Created attachment 103061 [details] Patch
Comment on attachment 103061 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103061&action=review > Source/WebKit2/UIProcess/WebInspectorProxy.cpp:221 > + my_isAttached = shouldOpenAttached(); my_isAttached -> m_isAttached
Created attachment 103064 [details] [PATCH] With typo fixed
Inspector stuff looks good, let's wait for WK2 experts to confirm that everything is OK.
Comment on attachment 103064 [details] [PATCH] With typo fixed View in context: https://bugs.webkit.org/attachment.cgi?id=103064&action=review > Source/WebKit2/UIProcess/win/WebInspectorProxyWin.cpp:259 > + return minimumAttachedHeight <= (inspectedWindowHeight * 3 / 4); The 3/4 here should be a constant like in the Mac code. But ideally we'd share this code between all platforms. We'd just need a cross-platform abstraction for getting the inspected window's height.
(In reply to comment #5) > (From update of attachment 103064 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=103064&action=review > > > Source/WebKit2/UIProcess/win/WebInspectorProxyWin.cpp:259 > > + return minimumAttachedHeight <= (inspectedWindowHeight * 3 / 4); > > The 3/4 here should be a constant like in the Mac code. But ideally we'd share this code between all platforms. We'd just need a cross-platform abstraction for getting the inspected window's height. I was thinking of doing the platformInspectedWindowHeight instead of the platformCanAttach, but then I thought that different platforms can have different considerations wrt preferred / minimum inspector size. Now sure which is better - flexibility or consistency in this case. One of the most popular requests on the inspector for chrome is do dock to side.
Comment on attachment 103064 [details] [PATCH] With typo fixed View in context: https://bugs.webkit.org/attachment.cgi?id=103064&action=review >>> Source/WebKit2/UIProcess/win/WebInspectorProxyWin.cpp:259 >>> + return minimumAttachedHeight <= (inspectedWindowHeight * 3 / 4); >> >> The 3/4 here should be a constant like in the Mac code. But ideally we'd share this code between all platforms. We'd just need a cross-platform abstraction for getting the inspected window's height. > > I was thinking of doing the platformInspectedWindowHeight instead of the platformCanAttach, but then I thought that different platforms can have different considerations wrt preferred / minimum inspector size. Now sure which is better - flexibility or consistency in this case. One of the most popular requests on the inspector for chrome is do dock to side. Since the Web Inspector is a feature of WebKit, not a feature of any particular browsers, I think consistency should be preferred.
Created attachment 103071 [details] [PATCH] Moved minimum size check & constants to WebInspectorProxy
Comment on attachment 103071 [details] [PATCH] Moved minimum size check & constants to WebInspectorProxy View in context: https://bugs.webkit.org/attachment.cgi?id=103071&action=review Should we change WebKit1 to use ?docked=true, too? The WebKit2 code looks fine to me. I'll let someone more familiar with the Inspector give the final r+. > Source/WebCore/ChangeLog:14 > + Web Inspector: implement dock/undock in WebKit2 without getting into WebCore. > + https://bugs.webkit.org/show_bug.cgi?id=65763 > + > + Reviewed by NOBODY (OOPS!). > + > + * WebCore.exp.in: > + * inspector/InspectorController.cpp: > + * inspector/InspectorController.h: > + * inspector/InspectorFrontendClient.h: > + * inspector/InspectorFrontendClientLocal.h: > + * inspector/front-end/inspector.js: > + (windowLoaded): You should explain what your patch is actually doing, and why. This makes it easier for reviewers and others to understand your changes. (Ditto for your other ChangeLogs.)
Created attachment 103076 [details] [PATCH] Patch with better changelog, for landing
(In reply to comment #10) > Created an attachment (id=103076) [details] > [PATCH] Patch with better changelog, for landing docked=true is a very nice way to handle this. Thanks for looking into it! I was going to ask if another solution would be to just port the "canAttach" to WebKit2, but you beat me to the punch. This looks good to me. If this lands, I can file a follow up bug to have the other WebKit ports switch over to docked=true. This looks good to me, but would be nice if an inspector person could r+.
Committed r92587: <http://trac.webkit.org/changeset/92587>