REGRESSION (r164702): Double tap doesn't stay under the new element once the animation finishes

• Component: WebKit2
• Status: RESOLVED FIXED

[Tim Horton]

The early return added to WebPage::scalePage is wrong because the origin can be different (and it only checks the scale), and also because it doesn't consult the PluginView's page scale.

<rdar://problem/16192842>

[Tim Horton]

Created attachment 230270 [details]
patch

[Tim Horton]

http://trac.webkit.org/changeset/167864

[Benjamin Poulain]

This fixes OS X but breaks iOS. :(

[Benjamin Poulain]

Ok, this breaks animated resize and the initial scale logic on iOS.

The patch is okay for OS X, we should split this for iOS IMHO.

[Tim Horton]

(In reply to comment #4)
> Ok, this breaks animated resize and the initial scale logic on iOS.
> 
> The patch is okay for OS X, we should split this for iOS IMHO.

What kind of split are you proposing? Also, does iOS just disregard the origin parameter?
Do we need to just additionally not do the dynamic size update/scaleFactorWasSetFromUIProcess bits if the scale doesn't change?

[Benjamin Poulain]

(In reply to comment #5)
> (In reply to comment #4)
> > Ok, this breaks animated resize and the initial scale logic on iOS.
> > 
> > The patch is okay for OS X, we should split this for iOS IMHO.
> 
> What kind of split are you proposing?

See https://bugs.webkit.org/show_bug.cgi?id=126022
It looks like WebPage::scalePage() and Page::setPageScaleFactor() do not work well when the scroll position is not handled by the ScrollView.

> Also, does iOS just disregard the origin parameter?

Yep, iOS uses delegateScrolling(), and Page::setPageScaleFactor() must ignore the scroll position or we will re-enter WebPage. Simon fixed that a while ago.

> Do we need to just additionally not do the dynamic size update/scaleFactorWasSetFromUIProcess bits if the scale doesn't change?

Yep, short term we should do that to unbreak iOS.
Longer term we should look into having a single coherent path for setting the scroll position from WebCore, in the same way that we have the coherent path for scale+scroll update from the UIProcess.

[Tim Horton]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Ok, this breaks animated resize and the initial scale logic on iOS.
> > > 
> > > The patch is okay for OS X, we should split this for iOS IMHO.
> > 
> > What kind of split are you proposing?
> 
> See https://bugs.webkit.org/show_bug.cgi?id=126022
> It looks like WebPage::scalePage() and Page::setPageScaleFactor() do not work well when the scroll position is not handled by the ScrollView.

Hmmpfh. Ok.

> > Also, does iOS just disregard the origin parameter?
> 
> Yep, iOS uses delegateScrolling(), and Page::setPageScaleFactor() must ignore the scroll position or we will re-enter WebPage. Simon fixed that a while ago.
> 
> > Do we need to just additionally not do the dynamic size update/scaleFactorWasSetFromUIProcess bits if the scale doesn't change?
> 
> Yep, short term we should do that to unbreak iOS.
> Longer term we should look into having a single coherent path for setting the scroll position from WebCore, in the same way that we have the coherent path for scale+scroll update from the UIProcess.

OK, I'll post a patch to just do that to unbreak iOS, for now. Reopening for that.

[Tim Horton]

Created attachment 230277 [details]
followup

[Tim Horton]

http://trac.webkit.org/changeset/167869

[Benjamin Poulain]

Thanks!