Bug 70459

Created attachment 111879 [details] Patch

With frameScaleFactor now always returning 1.0 for subframes and pageScaleFactor for the mainFrame, and there being only a single pageScaleFactor, history for scaling is broken. Scaling history is saved on a per frame basis but restored, overriding the per-page pageScaleFactor multiple times. As a result, sometimes pages that should be scaled end up getting a scale factor of 1.0. The easy solution is only restore pageScaleFactor for the main frame, but I'm not satisfied with this solution. We need to change the names of methods in HistoryItem to correspond to the reality that these are frameScaleFactors and not pageScaleFactors. But given that all frames except the main frame have a scale factor of 1.0, does it even make sense to store this information? Is there a global level History object where I should be storing pageScaleFactor?

Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.

Comment on attachment 111879 [details] Patch Attachment 111879 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10180704

Comment on attachment 111879 [details] Patch Attachment 111879 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/10182675

Comment on attachment 111879 [details] Patch Attachment 111879 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10183738

I think it is better to continue referring to this as page scale factor since that's what it is. That it is stored on HistoryItem doesn't change the fact that it still acts like a page scale factor. I think I'd just add a comment to HistoryItem indicating that the page scale factor field is ignored for subframes.

(In reply to comment #7) > I think it is better to continue referring to this as page scale factor since that's what it is. That it is stored on HistoryItem doesn't change the fact that it still acts like a page scale factor. I think I'd just add a comment to HistoryItem indicating that the page scale factor field is ignored for subframes. (In reply to comment #7) > I think it is better to continue referring to this as page scale factor since that's what it is. That it is stored on HistoryItem doesn't change the fact that it still acts like a page scale factor. I think I'd just add a comment to HistoryItem indicating that the page scale factor field is ignored for subframes. That's a fair short term solution. Ideally, I'd like to store the pageScaleFactor once per page. Simon Fraser told me to take a look at CachedPage to see if pageScaleFactor might belong there. I haven't looked at it yet, but I intend to investigate where would be a better place to store cached pages' pageScaleFactors... Maybe that clean up effort should be logged as a separate bug...

Created attachment 112369 [details] Patch

(In reply to comment #8) > That's a fair short term solution. Ideally, I'd like to store the pageScaleFactor once per page. Simon Fraser told me to take a look at CachedPage to see if pageScaleFactor might belong there. I haven't looked at it yet, but I intend to investigate where would be a better place to store cached pages' pageScaleFactors... > > Maybe that clean up effort should be logged as a separate bug... I'm confused by the CachedPage suggestion. (Chrome does not use CachedPage.) That is part of the back/forward caching feature, but even for ports that do enable the back/forward cache, there is not always a cached page. This is very different from session history (HistoryItem), which contains information required to re-create the page by loading the URL of the page, restoring form fields and adjusting scroll offsets.

Comment on attachment 112369 [details] Patch Is there anyway to create a layout test for this bug?

Comment on attachment 112369 [details] Patch Clearing flags on attachment: 112369 Committed r98453: <http://trac.webkit.org/changeset/98453>

All reviewed patches have been landed. Closing bug.