Bug 214886

Summary: Web Inspector: Don't allow docking to a hidden web view
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Timothy Hatcher
Reported 2020-07-28 11:15:50 PDT
When the Inspected Web View is hidden, don't try to attach.
Attachments
Patch (2.73 KB, patch)
2020-07-28 11:17 PDT, Timothy Hatcher 		no flags
DetailsFormatted DiffDiff
Patch (2.72 KB, patch)
2020-07-28 11:53 PDT, Timothy Hatcher 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Timothy Hatcher
Comment 1 2020-07-28 11:17:48 PDT
Created attachment 405380 [details] Patch
Devin Rousso
Comment 2 2020-07-28 11:40:55 PDT
Comment on attachment 405380 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405380&action=review r=me > Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:282 > + if (!m_inspectedPage || !m_inspectorPage || !platformCanAttach(m_canAttach)) NIT: can you use `platformCanAttach(canAttach())` instead? This is what other callers do, and it covers the future case where `canAttach` becomes more complex :) > Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:314 > + if (!m_inspectedPage || !m_inspectorPage || (!m_isAttached && m_isVisible)) Is `m_isVisible` necessary?
Timothy Hatcher
Comment 3 2020-07-28 11:53:37 PDT
Created attachment 405386 [details] Patch
Timothy Hatcher
Comment 4 2020-07-28 11:57:32 PDT
Comment on attachment 405380 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405380&action=review >> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:282 >> + if (!m_inspectedPage || !m_inspectorPage || !platformCanAttach(m_canAttach)) > > NIT: can you use `platformCanAttach(canAttach())` instead? This is what other callers do, and it covers the future case where `canAttach` becomes more complex :) Okay. >> Source/WebKit/UIProcess/Inspector/WebInspectorProxy.cpp:314 >> + if (!m_inspectedPage || !m_inspectorPage || (!m_isAttached && m_isVisible)) > > Is `m_isVisible` necessary? Yes, since m_isAttached could be stale after close.
EWS
Comment 5 2020-07-28 12:21:40 PDT
Committed r264999: <https://trac.webkit.org/changeset/264999> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405386 [details].
Radar WebKit Bug Importer
Comment 6 2020-07-28 12:22:19 PDT
<rdar://problem/66231117>
Note You need to log in before you can comment on or make changes to this bug.