Bug 128396

Summary: [iOS][WK2] Add empty definition for an API allowing clients to define obscured edges over the scrollview
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, commit-queue, ian, mitz, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Benjamin Poulain 2014-02-07 14:00:52 PST
[iOS][WK2] Add empty definition for an API allowing clients to define obscured edges over the scrollview
Comment 1 Benjamin Poulain 2014-02-07 14:01:56 PST
Created attachment 223494 [details]
Patch
Comment 2 WebKit Commit Bot 2014-02-07 14:04:35 PST
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.
Comment 3 Ian Henderson 2014-02-07 14:08:46 PST
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.
Comment 4 Anders Carlsson 2014-02-07 14:14:07 PST
(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.
Comment 5 Simon Fraser (smfr) 2014-02-07 14:32:06 PST
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.
Comment 6 Simon Fraser (smfr) 2014-02-07 14:45:25 PST
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:
Comment 7 Ian Henderson 2014-02-07 14:49:17 PST
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.
Comment 8 Benjamin Poulain 2014-02-07 15:19:14 PST
Created attachment 223502 [details]
Patch
Comment 9 WebKit Commit Bot 2014-02-07 15:20:51 PST
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.
Comment 10 Benjamin Poulain 2014-02-10 13:52:40 PST
Comment on attachment 223502 [details]
Patch

Clearing flags on attachment: 223502

Committed r163814: <http://trac.webkit.org/changeset/163814>
Comment 11 Benjamin Poulain 2014-02-10 13:52:42 PST
All reviewed patches have been landed.  Closing bug.