Summary: | Latched scrolling may interact badly with custom programmatic scrolling | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||||||||
Component: | Layout and Rendering | Assignee: | Brent Fulgham <bfulgham> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bfulgham, cmarcelo, commit-queue, dino, esprehn+autocc, kangil.han, simon.fraser | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 128225 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Brent Fulgham
2014-04-18 17:27:20 PDT
Created attachment 229692 [details]
Patch
smfr: Do we need logic to somehow clear the programmatic scroll state? If so, when would we decide to do so? Maybe at the beginning of a swipe event? Created attachment 229696 [details]
Patch
Created attachment 229710 [details]
Patch
Created attachment 229714 [details]
Patch
Comment on attachment 229714 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229714&action=review > Source/WebCore/dom/Element.cpp:802 > + if (RenderBox* rend = renderBox()) { rend is such an unpleasant local variable name; I suggest renderer instead > Source/WebCore/dom/Element.cpp:804 > + if (ScrollableArea* scrollableArea = static_cast<ScrollableArea*>(rend->layer())) This typecast is a bad idea. A RenderLayer is a ScrollableArea, so there is no reason to cast, but further, no reason to narrow the type like this. It should just say: if (auto* layer = renderer->layer()) layer->setScrolledProgrammatically(true); > Source/WebCore/dom/Element.cpp:805 > + scrollableArea->setScrolledProgrammatically(true); This won’t ever get reset to false? It’s a permanent state of affairs? > Source/WebCore/page/EventHandler.h:301 > +protected: > + void clearLatchedState(); should be private not protected; no classes derive from EventHandler, so there’s no difference between protected and private Also, should always prefer private to protected unless there is some reason a member needs to be exposed to derived classes. Committed r167560: <http://trac.webkit.org/changeset/167560> Created attachment 407569 [details]
Testcase
|