rdar://80113612
Created attachment 433291 [details] For EWS
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?
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.
Created attachment 433316 [details] Patch
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].
(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.