| Summary: | Add a property on WKWebView to determine if it is being inspected by Web Inspector | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Test <cehiti9541> | ||||||||||||
| Component: | Web Inspector | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | bburg, eepskamphunt, eepskamp, eric.carlson, hi, inspector-bugzilla-changes, katherine_cheney | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Test
2020-10-12 12:43:50 PDT
Created attachment 411149 [details]
Patch
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? 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 (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 :) Created attachment 411168 [details]
Patch
Comment on attachment 411168 [details]
Patch
r=me
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 Created attachment 411169 [details]
Patch
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.
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. Created attachment 411243 [details]
Patch
Created attachment 411255 [details]
Patch
(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. Comment on attachment 411255 [details]
Patch
r=me
Committed r268428: <https://trac.webkit.org/changeset/268428> All reviewed patches have been landed. Closing bug and clearing flags on attachment 411255 [details]. |