Bug 130709 – [iOS] Inspector View Indication Support

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Joseph Pecoraro]

iOS view indication for the remote web inspector needs to be implemented.

On incoming Indicate RWI messages, we should show the typical blue highlight over the WKWebView.

<rdar://problem/16415035>

[Joseph Pecoraro]

Created attachment 227716 [details]
[PATCH] Proposed Fix

[Simon Fraser (smfr)]

Comment on attachment 227716 [details]
[PATCH] Proposed Fix

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1097
> +- (void)_startIndicatingView

"indicatingView" is ambiguous and not really good English. It should be read as -(UIView*)indicatingView, but I think you mean it as the view is "indicating the state of being inspected".

How about showShowingInspectionIndication/indicator? Or -(void)setIsShowingInspectionIndicator:(BOOL)?

> Source/WebKit2/UIProcess/PageClient.h:255
> +    virtual void startIndicatingView() = 0;
> +    virtual void stopIndicatingView() = 0;

Same bad naming.

> Source/WebKit2/UIProcess/ios/WKContentView.mm:64
> +@interface IndicateView : UIView

I think this class name should contain "inspector". It should have a prefix (WK or Web?), maybe with an underscore. Web/WKInspectorIndicationView?

[Simon Fraser (smfr)]

I'm OK with the patch, BTW, but I do object to the naming.

[Joseph Pecoraro]

Created attachment 227780 [details]
[PATCH] Renamed methods and classes

[WebKit Commit Bot]

Attachment 227780 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentView.h:56:  Missing spaces around =  [whitespace/operators] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 227780 [details]
[PATCH] Renamed methods and classes

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

> Source/WebKit2/UIProcess/WebPageProxy.h:1325
> +    void startIndicating();
> +    void stopIndicating();

"indicating" is still too vague here. There may be many kinds of indication, of which Inspector overlays may be just one.

> Source/WebKit2/WebProcess/WebPage/WebPage.h:472
> +    void indicate();
> +    void hideIndication();

Also too vague, and annoyingly different terminology to that on PageClient.

[Joseph Pecoraro]

Created attachment 227787 [details]
[PATCH] More renames

[WebKit Commit Bot]

Attachment 227787 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentView.h:56:  Missing spaces around =  [whitespace/operators] [4]
Total errors found: 1 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 227787 [details]
[PATCH] More renames

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

> Source/WebKit/mac/WebView/WebView.mm:1896
> +- (void)setShowingInspectorIndication:(BOOL)enabled

enabled -> showing?

[Tim Horton]

This landed in http://trac.webkit.org/changeset/166257 as far as I can tell.

[Joseph Pecoraro]

Yes it did, thanks!