Bug 198656

Summary: REGRESSION (r245006): Setting scrollview.scrollEnabled clobbers any scrollEnabled set by a client
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: ScrollingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fred.wang, simon.fraser, thorton, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=153852
Attachments:
Description Flags
patch
fred.wang: review+
with zooming fix too none

Description Antti Koivisto 2019-06-07 05:17:26 PDT 
Remember user setting separately.
Comment 1 Antti Koivisto 2019-06-07 05:18:12 PDT 
<rdar://problem/51494585>
Comment 2 Antti Koivisto 2019-06-07 05:23:38 PDT 
Created attachment 371582 [details]
patch
Comment 3 Tim Horton 2019-06-07 10:30:48 PDT 
Comment on attachment 371582 [details]
patch

We need to do something similar for zoomEnabled and a bunch of other properties, but this is a good start (and fixes a regression, where the others have been broken for ages).
Comment 4 Antti Koivisto 2019-06-07 10:54:01 PDT 
If this pattern looks ok I can do zoom too here as it is basically the same thing.
Comment 5 Simon Fraser (smfr) 2019-06-07 11:06:40 PDT 
Devs have also complained about scroll-snap code overriding the deceleration values.
Comment 6 Wenson Hsieh 2019-06-07 11:18:08 PDT 
(In reply to Simon Fraser (smfr) from comment #5)
> Devs have also complained about scroll-snap code overriding the deceleration
> values.

Hm...could've sworn I fixed this in r188541 :/
Comment 7 Wenson Hsieh 2019-06-07 11:19:39 PDT 
(In reply to Wenson Hsieh from comment #6)
> (In reply to Simon Fraser (smfr) from comment #5)
> > Devs have also complained about scroll-snap code overriding the deceleration
> > values.
> 
> Hm...could've sworn I fixed this in r188541 :/

Ah, maybe r211197 undid this...
Comment 8 Tim Horton 2019-06-07 11:38:42 PDT 
(In reply to Antti Koivisto from comment #4)
> If this pattern looks ok I can do zoom too here as it is basically the same
> thing.

Please do! The number of times I've seen people trying to override viewForZooming to achieve this and then hosing everything is v. high (inspiring https://trac.webkit.org/changeset/228253/webkit).
Comment 9 Antti Koivisto 2019-06-07 13:01:47 PDT 
Created attachment 371604 [details]
with zooming fix too
Comment 10 WebKit Commit Bot 2019-06-07 22:05:51 PDT 
Comment on attachment 371604 [details]
with zooming fix too

Clearing flags on attachment: 371604

Committed r246230: <https://trac.webkit.org/changeset/246230>
Comment 11 WebKit Commit Bot 2019-06-07 22:05:53 PDT 
All reviewed patches have been landed.  Closing bug.