There are a number of problems for docking behaviour on Windows. For starters it does not ever constrain the inspectors size properly while docked. It also does not properly set the whether or not the inspector can be docked/undocked.
Created attachment 173710 [details] patch
Comment on attachment 173710 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=173710&action=review > Source/WebCore/inspector/InspectorFrontendClientLocal.cpp:143 > + // Call setDockingUnavailable before bringToFront. If we display the inspector's HWND first it causes the call to canAttachWindow to return the wrong result. I wonder if you can talk about this in a more generic way. Using the word HWND in cross-platform code is weird (at the very least, note that this is Windows only). Also, what/why does it cause canAttachWindow to return the wrong result? > Source/WebKit/win/ChangeLog:8 > + There are a number of problems for docking behaviour on Windows. s/for/with/ > Source/WebKit/win/ChangeLog:9 > + For starters it does not ever constrain the inspectors size properly while docked. s/For starters/For starters,/ s/inspectors/inspector's/ > Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:296 > + // We need to set the attached windows height before we actually attach the window. s/windows/window's/ > Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:297 > + // Make sure that m_attached is true so that setAttachedWindowHeight succeeds. s/succeeds/doesn't return early/?
Comment on attachment 173710 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=173710&action=review I'd like to see one more version with Tim's comments and mine addressed. Patch looks good though. > Source/WebKit/win/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=100000 100000!!! > Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:258 > + restoreAttachedWindowHeight(); A comment explaining why we are restoring the attached window height before attaching would be nice. >> Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:297 >> + // Make sure that m_attached is true so that setAttachedWindowHeight succeeds. > > s/succeeds/doesn't return early/? Should this say restoreAttachedWindowHeight instead of setAttachedWindowHeight?
> >> Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:297 > >> + // Make sure that m_attached is true so that setAttachedWindowHeight succeeds. > > > > s/succeeds/doesn't return early/? > > Should this say restoreAttachedWindowHeight instead of setAttachedWindowHeight? I meant setAttachedWindowHeight because restore calls setAttachedWindowHeight which has a check: if (!m_attached) return; which prevents the final height from being et properly. I guess both are correct but I figured it'd be a little clearer using the method that actually fails. Do you think I should make it restoreAttachedWindowHeight tho?
Created attachment 173746 [details] revised patch
Comment on attachment 173746 [details] revised patch View in context: https://bugs.webkit.org/attachment.cgi?id=173746&action=review > Source/WebCore/inspector/InspectorFrontendClientLocal.cpp:143 > + // Call setDockingUnavailable before bringToFront. If we display the inspector window via bringToFront first it causes the call to canAttachWindow to return the wrong result. Another sentence on why this is the case would be helpful. > Source/WebKit/win/ChangeLog:15 > + (WebInspectorFrontendClient::frontendLoaded): > + (WebInspectorFrontendClient::attachWindow): A high level comment about the changes in these functions would nice for people reading the ChangeLog later. > Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:299 > + // Immediately after calling showWindowWithoutNotifications(), the parent frameview's visibleHeight incorrectly returns 0 always. Is this a WebKit bug or a Windows bug?
Committed: http://trac.webkit.org/changeset/134327 And made the last few remaining changes. > Is this a WebKit bug or a Windows bug? Yeup, windows bug. I added a comment or two to clarify
Closing since it looks like this landed.