Bug 128396 – [iOS][WK2] Add empty definition for an API allowing clients to define obscured edges over the scrollview

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[iOS][WK2] Add empty definition for an API allowing clients to define obscured edges over the scrollview

[Benjamin Poulain]

Created attachment 223494 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:48:  Missing spaces around =  [whitespace/operators] [4]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:40:  Missing spaces around =  [whitespace/operators] [4]
Total errors found: 2 in 5 files


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

[Ian Henderson]

Comment on attachment 223494 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:48
> +@property (nonatomic, setter=_setObscuredInset:) UIEdgeInsets _obscuredInset;

I think the usual UIKit style for private properties is to omit the underscore from the property name and define both setter=_setProperty: and getter=_property.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:374
> +- (UIEdgeInsets)_obscuredInset
> +{
> +    return _obscuredInset;
> +}

If we don't define this method, the compiler should autosynthesize it and the ivar for us.

[Anders Carlsson]

(In reply to comment #3)
> (From update of attachment 223494 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223494&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:48
> > +@property (nonatomic, setter=_setObscuredInset:) UIEdgeInsets _obscuredInset;
> 
> I think the usual UIKit style for private properties is to omit the underscore from the property name and define both setter=_setProperty: and getter=_property.

We've decided against doing that for properties. If we decide to make a private property public with slightly different semantics we don't want clients to break.

[Simon Fraser (smfr)]

Comment on attachment 223494 [details]
Patch

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

> Source/WebKit2/UIProcess/API/ios/WKViewIOS.mm:276
> +- (void)_commitObscuredInset
> +{
> +}

Not sure why committing is separate from setting.

[Simon Fraser (smfr)]

Comment on attachment 223494 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:376
> +- (void)_setObscuredInset:(UIEdgeInsets)obscuredInset

_setObscuredInsets

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

Need a better name for this, or use a flag on -_setObscuredInsets:

[Ian Henderson]

Comment on attachment 223494 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:51
> +// Inform the web content that the unobscured area has reached a stable state.
> +- (void)_commitObscuredInset;

Ben and I discussed this, and we think it makes sense to have a pair of methods:
- (void)beginInteractiveObscuredInsetChange;
- (void)endInteractiveObscuredInsetChange;
When every begin call has been balanced by an end call, we'll inform the web content of the change.

[Benjamin Poulain]

Created attachment 223502 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:48:  Missing spaces around =  [whitespace/operators] [4]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:40:  Missing spaces around =  [whitespace/operators] [4]
Total errors found: 2 in 5 files


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

[Benjamin Poulain]

Comment on attachment 223502 [details]
Patch

Clearing flags on attachment: 223502

Committed r163814: <http://trac.webkit.org/changeset/163814>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.