Bug 179125 – Use more generic names than "overflow" for functions that can be used for subframes

• Component: Frames
• Status: RESOLVED FIXED

[Frédéric Wang (:fredw)]

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).

[Frédéric Wang (:fredw)]

Created attachment 325586 [details]
WIP Patch

[Frédéric Wang (:fredw)]

Created attachment 325892 [details]
Patch

[Frédéric Wang (:fredw)]

(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.

[Frédéric Wang (:fredw)]

Created attachment 340075 [details]
Patch

Rebasing...

[Frédéric Wang (:fredw)]

Created attachment 348914 [details]
Patch

Rebasing...

[Simon Fraser (smfr)]

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"

[Frédéric Wang (:fredw)]

Created attachment 349032 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 349032 [details]
Patch for landing

Clearing flags on attachment: 349032

Committed r235741: <https://trac.webkit.org/changeset/235741>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44185073>