Summary: | [iOS][WK2] Restore the scroll position and scale from the HistoryItem (mostly) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||
Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | simon.fraser | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Benjamin Poulain
2014-04-28 15:30:49 PDT
Created attachment 230328 [details]
Patch
Comment on attachment 230328 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230328&action=review > Source/WebKit2/WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:90 > + if (page && m_frame->isMainFrame() && currentItem->pageScaleFactor()) > + m_frame->page()->restorePageState(currentItem->pageScaleFactor(), currentItem->scrollPoint()); Is the currentItem->pageScaleFactor() test useful, or just trying to catch odd uninitialized cases? > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:144 > + FrameView& frameView = *m_page->mainFrame().view(); > + frameView.setScrollPosition(scrollPosition); Odd to put FrameView in a local when you only use it once. > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:145 > + m_userHasChangedPageScaleFactor = true; Why? Add a comment? (In reply to comment #2) > (From update of attachment 230328 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=230328&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:90 > > + if (page && m_frame->isMainFrame() && currentItem->pageScaleFactor()) > > + m_frame->page()->restorePageState(currentItem->pageScaleFactor(), currentItem->scrollPoint()); > > Is the currentItem->pageScaleFactor() test useful, or just trying to catch odd uninitialized cases? Zero is the default value of HistoryItem. It is a weird API, I would prefer if HistoryItem would keep a separate state to know if values have been set. Relying on the default pageScaleFactor is odd. Created attachment 230345 [details]
Patch
Committed r167916: <http://trac.webkit.org/changeset/167916> Comment on attachment 230345 [details]
Patch
Uploaded this again by accident.
|