| Summary: | Swipe snapshot removed too early (jumps around) on arstechnica and NYT | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
| Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | andersca, mitz, sam, simon.fraser | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Horton
2015-03-29 14:37:21 PDT
Created attachment 249700 [details]
Patch
Comment on attachment 249700 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249700&action=review > Source/WebKit2/ChangeLog:3 > + REGRESSION: Swipe snapshot removed too early (jumps around) on arstechnica and NYT Regression or not a regression? Comment on attachment 249700 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249700&action=review > Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:151 > + , m_swipeWaitingForRenderTreeSizeThreshold(false) > + , m_swipeWaitingForRepaint(false) > + , m_swipeWaitingForDidFinishLoad(false) > + , m_swipeWaitingForSubresourceLoads(false) > + , m_swipeWaitingForScrollPositionRestoration(false) Should initialize these in the header instead of the constructor. Should do the same for other scalars that are being initialized to an “obvious” default such as “false” or “0”. Helps keep changes in the different per-platform .mm files to a minimum and makes it harder to forget to initialize one. Not a regression in the WebKit sense. Will remove from the changelog (I realized after I uploaded the patch). (In reply to comment #3) > Comment on attachment 249700 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=249700&action=review > > > Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:151 > > + , m_swipeWaitingForRenderTreeSizeThreshold(false) > > + , m_swipeWaitingForRepaint(false) > > + , m_swipeWaitingForDidFinishLoad(false) > > + , m_swipeWaitingForSubresourceLoads(false) > > + , m_swipeWaitingForScrollPositionRestoration(false) > > Should initialize these in the header instead of the constructor. Should do > the same for other scalars that are being initialized to an “obvious” > default such as “false” or “0”. Helps keep changes in the different > per-platform .mm files to a minimum and makes it harder to forget to > initialize one. Yes! Will fix. Thanks! Created attachment 249706 [details]
Patch for landing
|