Bug 227859 – [watchOS] Make a few additional adjustments to support system minimum layout margins

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Wenson Hsieh]

rdar://80113612

[Wenson Hsieh]

Created attachment 433291 [details]
For EWS

[Tim Horton]

Comment on attachment 433291 [details]
For EWS

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

> Source/WebKit/UIProcess/ios/WKScrollView.mm:464
> +        super.contentScrollInset = UIEdgeInsetsZero;

Is this bit going to cause any trouble on iOS?

[Wenson Hsieh]

Comment on attachment 433291 [details]
For EWS

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

Thanks for the review!

>> Source/WebKit/UIProcess/ios/WKScrollView.mm:464
>> +        super.contentScrollInset = UIEdgeInsetsZero;
> 
> Is this bit going to cause any trouble on iOS?

Oh, good catch! I think that in theory, it could cause trouble if a client were setting `-_contentScrollInset` on iOS (since the interaction between `-_contentScrollInset` and `-contentInset` is…reasonable on non-watch iOS platforms). Since the need to duck `-_contentScrollInset` when a `-contentInset` is specified is really only needed on watchOS, I'll make some of this code conditional for watchOS only.

That said — as this patch currently is, this method shouldn't be reachable at all on non-watchOS platforms, since the only way it can be reached is for `_contentScrollInsetInternal` to be set, and we only call `-_setContentScrollInsetInternal:` from a watchOS-specific codepath.

[Wenson Hsieh]

Created attachment 433316 [details]
Patch

[EWS]

Committed r279830 (239591@main): <https://commits.webkit.org/239591@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433316 [details].

[Tim Horton]

(In reply to Wenson Hsieh from comment #3)
> Comment on attachment 433291 [details]
> For EWS
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433291&action=review
> 
> Thanks for the review!
> 
> >> Source/WebKit/UIProcess/ios/WKScrollView.mm:464
> >> +        super.contentScrollInset = UIEdgeInsetsZero;
> > 
> > Is this bit going to cause any trouble on iOS?
> 
> Oh, good catch! I think that in theory, it could cause trouble if a client
> were setting `-_contentScrollInset` on iOS (since the interaction between
> `-_contentScrollInset` and `-contentInset` is…reasonable on non-watch iOS
> platforms). Since the need to duck `-_contentScrollInset` when a
> `-contentInset` is specified is really only needed on watchOS, I'll make
> some of this code conditional for watchOS only.

Perfect! Thank you. Best to limit the counter-hacks to places where the hacks exist :)

> That said — as this patch currently is, this method shouldn't be reachable
> at all on non-watchOS platforms, since the only way it can be reached is for
> `_contentScrollInsetInternal` to be set, and we only call
> `-_setContentScrollInsetInternal:` from a watchOS-specific codepath.

Sure, makes sense.