Bug 130273 – Implement contentInset for Mac WebKit2

• Component: WebKit API
• Status: RESOLVED FIXED

[Beth Dakin]

Implement contentInset for Mac WebKit2

<rdar://problem/16175195>

[Beth Dakin]

Created attachment 227319 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/testing/Internals.h:257:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:275:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:689:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:691:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:1093:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:1095:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 6 in 26 files


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

[Simon Fraser (smfr)]

Comment on attachment 227319 [details]
Patch

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

> Source/WebCore/page/Page.h:282
> +    void setTopContentInset(float contentInset) { m_topContentInset = contentInset; }

Doesn't this need to cause layout or other rejiggery?

> Source/WebCore/platform/ScrollView.cpp:280
> +        return visibleCOntentSize;

visibleCOntentSize

> Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:84
> +@property (readwrite) CGFloat topContentInset;

Should be on WKWebView.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:3303
> +- (void)setTopContentInset:(CGFloat)contentInset
> +{
> +    _data->_page->setTopContentInset(contentInset);
> +}
> +
> +- (CGFloat)topContentInset
> +{
> +    return _data->_page->topContentInset();
> +}

We're not using WKView any more

[Beth Dakin]

(In reply to comment #3)
> (From update of attachment 227319 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227319&action=review
> 
> > Source/WebCore/page/Page.h:282
> > +    void setTopContentInset(float contentInset) { m_topContentInset = contentInset; }
> 
> Doesn't this need to cause layout or other rejiggery?
> 

Yes, fixing.

> > Source/WebCore/platform/ScrollView.cpp:280
> > +        return visibleCOntentSize;
> 
> visibleCOntentSize
> 

Oops!

> > Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:84
> > +@property (readwrite) CGFloat topContentInset;
> 
> Should be on WKWebView.
> 
> > Source/WebKit2/UIProcess/API/mac/WKView.mm:3303
> > +- (void)setTopContentInset:(CGFloat)contentInset
> > +{
> > +    _data->_page->setTopContentInset(contentInset);
> > +}
> > +
> > +- (CGFloat)topContentInset
> > +{
> > +    return _data->_page->topContentInset();
> > +}
> 
> We're not using WKView any more

My new patch adds it to both.

[Beth Dakin]

Created attachment 227337 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/page/Page.h:282:  The parameter name "contentInset" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:97:  Missing spaces around =  [whitespace/operators] [4]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:84:  Missing spaces around =  [whitespace/operators] [4]
ERROR: Source/WebCore/platform/ScrollView.cpp:689:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:691:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:1093:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:1095:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 7 in 29 files


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

[Beth Dakin]

Created attachment 227340 [details]
Patch

Stylebot is just wrong about the WK2 setter things, but here is a patch that will hopefully placate the other complaints.

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:97:  Missing spaces around =  [whitespace/operators] [4]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:84:  Missing spaces around =  [whitespace/operators] [4]
ERROR: Source/WebCore/platform/ScrollView.cpp:689:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/ScrollView.cpp:1093:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 4 in 29 files


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

[Beth Dakin]

Created attachment 227346 [details]
Patch

Once again for stylebot.

[WebKit Commit Bot]

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


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


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

[Beth Dakin]

Thanks, Simon! 

http://trac.webkit.org/changeset/166017

[Alexey Proskuryakov]

Comment on attachment 227346 [details]
Patch

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

> Source/WebCore/testing/Internals.cpp:1831
> +void Internals::setTopContentInset(float contentInset)

Is there something resetting the inset between tests?

[Simon Fraser (smfr)]

This broke tests because
ScrollView::visibleContentRectInternal(...IncludeScollbars) calls unobscuredContentRect() which excludes them.

[Simon Fraser (smfr)]

Fixed in http://trac.webkit.org/changeset/166032