Bug 217629 – Add a property on WKWebView to determine if it is being inspected by Web Inspector

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Test]

If you are inspecting a webview, the property will be true.

[Ellie]

rdar://70073369

[Ellie]

Created attachment 411149 [details]
Patch

[Kate Cheney]

Comment on attachment 411149 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411149&action=review

> Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm:1656
> +- (BOOL)_isBeingInspected

I am not sure you need this in both WKWebView and WKWebViewMac. Is there a reason you put it in both?

[Devin Rousso]

Comment on attachment 411149 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411149&action=review

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:579
> +@property (nonatomic, readonly) BOOL _isBeingInspected WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));

I'd put this right above the `_inspector`, as that will cover both macOS and iOS

I'd do the same with the implementation in `Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm` too

[Ellie]

(In reply to katherine_cheney from comment #3)
> Comment on attachment 411149 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411149&action=review
> 
> > Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm:1656
> > +- (BOOL)_isBeingInspected
> 
> I am not sure you need this in both WKWebView and WKWebViewMac. Is there a
> reason you put it in both?

You are right, I don't need it in both! My reason? I was confused :)

[Ellie]

Created attachment 411168 [details]
Patch

[Devin Rousso]

Comment on attachment 411168 [details]
Patch

r=me

[Devin Rousso]

Comment on attachment 411168 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411168&action=review

> Source/WebKit/ChangeLog:13
> +        * UIProcess/API/mac/WKWebViewMac.mm:
> +        (-[WKWebView _isBeingInspected]):

oh btw these lines should be removed :P

[Ellie]

Created attachment 411169 [details]
Patch

[Blaze Burg]

Comment on attachment 411169 [details]
Patch

I'm in favor of this SPI, though the name seems inelegant. (We don't use 'Being' much in Cocoa API).

Please remove the now-redundant _hasInspectorFrontend from WKWebViewPrivateForTesting and its uses in the WKInspectorDelegate API test.

[Eric Carlson]

Comment on attachment 411169 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411169&action=review

This should have a test.

> Source/WebKit/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).
> +

It would be helpful to have a comment about why the is needed.

[Ellie Epskamp-Hunt]

Created attachment 411243 [details]
Patch

[Ellie Epskamp-Hunt]

Created attachment 411255 [details]
Patch

[Ellie Epskamp-Hunt]

(In reply to Eric Carlson from comment #11)
> Comment on attachment 411169 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411169&action=review
> 
> This should have a test.
> 
> > Source/WebKit/ChangeLog:8
> > +        Reviewed by NOBODY (OOPS!).
> > +
> 
> It would be helpful to have a comment about why the is needed.

I updated the existing tests and added a comment about why the property is needed.

[Blaze Burg]

Comment on attachment 411255 [details]
Patch

r=me

[EWS]

Committed r268428: <https://trac.webkit.org/changeset/268428>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411255 [details].