WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
132307
[iOS][WK2] Restore the scroll position and scale from the HistoryItem (mostly)
https://bugs.webkit.org/show_bug.cgi?id=132307
Summary
[iOS][WK2] Restore the scroll position and scale from the HistoryItem (mostly)
Benjamin Poulain
Reported
2014-04-28 15:30:49 PDT
[iOS][WK2] Restore the scroll position and scale from the HistoryItem (mostly)
Attachments
Patch
(12.39 KB, patch)
2014-04-28 15:37 PDT
,
Benjamin Poulain
no flags
Details
Formatted Diff
Diff
Patch
(11.68 KB, patch)
2014-04-28 18:20 PDT
,
Benjamin Poulain
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Benjamin Poulain
Comment 1
2014-04-28 15:37:56 PDT
Created
attachment 230328
[details]
Patch
Simon Fraser (smfr)
Comment 2
2014-04-28 16:20:16 PDT
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?
Benjamin Poulain
Comment 3
2014-04-28 17:49:26 PDT
(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.
Benjamin Poulain
Comment 4
2014-04-28 18:20:27 PDT
Created
attachment 230345
[details]
Patch
Benjamin Poulain
Comment 5
2014-04-28 18:31:12 PDT
Committed
r167916
: <
http://trac.webkit.org/changeset/167916
>
Benjamin Poulain
Comment 6
2014-04-28 18:31:57 PDT
Comment on
attachment 230345
[details]
Patch Uploaded this again by accident.
Simon Fraser (smfr)
Comment 7
2014-07-24 11:50:05 PDT
<
rdar://problem/16031704
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug