Bug 179125

Summary: Use more generic names than "overflow" for functions that can be used for subframes
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: FramesAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fred.wang, rbuis, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=179172
Bug Depends on:    
Bug Blocks: 149264    
Attachments:
Description Flags
WIP Patch
none
Patch
none
Patch
none
Patch
simon.fraser: review+
Patch for landing none

Description Frédéric Wang (:fredw) 2017-11-01 10:24:15 PDT
The following functions might need to be used by subframes to implement iframe scrolling on iOS (bug 149264). I'm not really sure about the best name to describe "overflow and subframes", but the name "overflow" might not be quite correct.

These are currently called when the current patch for bug 173833 is used:

PageClient::overflowScrollViewWillStartPanGesture()
PageClient::overflowScrollViewDidScroll()
PageClient::overflowScrollWillStartScroll()
PageClient::overflowScrollDidEndScroll()

I suspect we need to implement FrameView::didStartScroll, FrameView::didEndScroll (and  FrameView::didUpdateScroll) which would require the following functions too:


ChromeClient::didStartOverflowScroll()
ChromeClient::didEndOverflowScroll()

This ends up calling the following, but I'm not sure where they are used. I suspect they are meant to be overriden somewhere outside WebKit so I'm not sure we can just rename them:

UITextInteractionAssistant::willStartScrollingOverflow
UITextInteractionAssistant::willEndScrollingOverflow
UIWebSelectionAssistant::willStartScrollingOverflow
UIWebSelectionAssistant::willEndScrollingOverflow
WebUIKitDelegate::webkitViewDidStartOverflowScroll
WebUIKitDelegate::webkitViewDidEndOverflowScroll

I'm not yet sure about EditorClient::overflowScrollPositionChange (currently called from AsyncScrollingCoordinator::updateScrollPositionAterAsyncScroll).
Comment 1 Frédéric Wang (:fredw) 2017-11-01 10:26:00 PDT
Created attachment 325586 [details]
WIP Patch
Comment 2 Frédéric Wang (:fredw) 2017-11-03 07:04:54 PDT
Created attachment 325892 [details]
Patch
Comment 3 Frédéric Wang (:fredw) 2017-11-03 07:08:08 PDT
(In reply to Frédéric Wang (:fredw) from comment #0)
> PageClient::overflowScrollViewWillStartPanGesture()
> PageClient::overflowScrollViewDidScroll()
> PageClient::overflowScrollWillStartScroll()
> PageClient::overflowScrollDidEndScroll()

I uploaded a patch for these.

> 
> I suspect we need to implement FrameView::didStartScroll,
> FrameView::didEndScroll (and  FrameView::didUpdateScroll) which would
> require the following functions too:
> 
> 
> ChromeClient::didStartOverflowScroll()
> ChromeClient::didEndOverflowScroll()

Not sure whether they are needed for frame view but if that's the case, the renaming can be handled in 179172. 

> I'm not yet sure about EditorClient::overflowScrollPositionChange (currently
> called from AsyncScrollingCoordinator::updateScrollPositionAterAsyncScroll).

Still not clear what's the effect of that one and whether we need it for subframe scrolling. There is also another overflowScrollPositionChangedForNode function which is only used in WebKitLegacy.
Comment 4 Frédéric Wang (:fredw) 2018-05-09 23:44:34 PDT
Created attachment 340075 [details]
Patch

Rebasing...
Comment 5 Frédéric Wang (:fredw) 2018-09-05 07:07:31 PDT
Created attachment 348914 [details]
Patch

Rebasing...
Comment 6 Simon Fraser (smfr) 2018-09-05 11:08:55 PDT
Comment on attachment 348914 [details]
Patch

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

> Source/WebKit/ChangeLog:10
> +        use a more generic "scrollling node" name.

"scrollling"
Comment 7 Frédéric Wang (:fredw) 2018-09-06 09:18:10 PDT
Created attachment 349032 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2018-09-06 09:56:22 PDT
Comment on attachment 349032 [details]
Patch for landing

Clearing flags on attachment: 349032

Committed r235741: <https://trac.webkit.org/changeset/235741>
Comment 9 WebKit Commit Bot 2018-09-06 09:56:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-09-06 09:58:31 PDT
<rdar://problem/44185073>