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>
Created attachment 230270 [details] patch
http://trac.webkit.org/changeset/167864
This fixes OS X but breaks iOS. :(
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.
(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?
(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.
(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.
Created attachment 230277 [details] followup
http://trac.webkit.org/changeset/167869
Thanks!