Bug 179125 - Use more generic names than "overflow" for functions that can be used for subframes
Summary: Use more generic names than "overflow" for functions that can be used for sub...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords: InRadar
Depends on:
Blocks: 149264
  Show dependency treegraph
 
Reported: 2017-11-01 10:24 PDT by Frédéric Wang (:fredw)
Modified: 2018-09-06 09:58 PDT (History)
5 users (show)

See Also:


Attachments
WIP Patch (17.27 KB, patch)
2017-11-01 10:26 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (15.37 KB, patch)
2017-11-03 07:04 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (15.90 KB, patch)
2018-05-09 23:44 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (16.02 KB, patch)
2018-09-05 07:07 PDT, Frédéric Wang (:fredw)
simon.fraser: review+
Details | Formatted Diff | Diff
Patch for landing (16.02 KB, patch)
2018-09-06 09:18 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>