Chromium Code Reviews Chromium Code Reviews
Issue 1895323002:
Viewport apply scroll should be on the document element not scrollingElement. (Closed)
Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: third_party/WebKit/Source/core/dom/Document.cpp |
| diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp |
| index 5ba449da09f6838f13fb7d2f535a713d92be2708..7a0797743b2b05458b93b86eee9111e73c563782 100644 |
| --- a/third_party/WebKit/Source/core/dom/Document.cpp |
| +++ b/third_party/WebKit/Source/core/dom/Document.cpp |
| @@ -584,6 +584,8 @@ void Document::childrenChanged(const ChildrenChange& change) |
| { |
| ContainerNode::childrenChanged(change); |
| m_documentElement = ElementTraversal::firstWithin(*this); |
| + |
| + updateViewportApplyScroll(); |
| } |
| void Document::updateViewportApplyScroll() |
|
tdresser
2016/04/19 17:48:42
We could put this in an anonymous namespace, and h
bokan
2016/04/19 18:36:27
Done.
|
| @@ -593,38 +595,18 @@ void Document::updateViewportApplyScroll() |
| || ownerElement()) |
| return; |
| - Element* newScrollingElement = scrollingElement(); |
| - |
| - // If there is no scrolling element (in QuirksMode and body is scrollable), |
| - // install the viewport scroll callback on the <HTML> element. |
| - if (!newScrollingElement) |
| - newScrollingElement = m_documentElement; |
| - |
| - if (newScrollingElement == m_oldScrollingElement) |
| + if (m_documentElement->getApplyScroll()) |
| return; |
| - ScrollStateCallback* applyScroll = nullptr; |
| - |
| - // If the scrolling element changed, remove the apply scroll from the |
| - // old one and keep it to put on the new scrolling element. |
| - if (m_oldScrollingElement) { |
| - applyScroll = m_oldScrollingElement->getApplyScroll(); |
| - m_oldScrollingElement->removeApplyScroll(); |
| - } |
| + ScrollStateCallback* applyScroll = |
| + new ViewportScrollCallback(*this, *frameHost()); |
| - if (newScrollingElement) { |
| - if (!applyScroll) |
| - applyScroll = new ViewportScrollCallback(*this, *frameHost()); |
| - |
| - // Use disable-native-scroll since the ViewportScrollCallback needs to |
| - // apply scroll actions before (TopControls) and after (overscroll) |
| - // scrolling the element so it applies scroll to the element itself. |
| - newScrollingElement->setApplyScroll( |
| - applyScroll, |
| - "disable-native-scroll"); |
| - } |
| - |
| - m_oldScrollingElement = newScrollingElement; |
| + // Use disable-native-scroll since the ViewportScrollCallback needs to |
| + // apply scroll actions before (TopControls) and after (overscroll) |
| + // scrolling the element so it applies scroll to the element itself. |
| + m_documentElement->setApplyScroll( |
| + applyScroll, |
| + "disable-native-scroll"); |
| } |
| AtomicString Document::convertLocalName(const AtomicString& name) |
| @@ -1905,8 +1887,6 @@ void Document::updateLayout() |
| void Document::layoutUpdated() |
| { |
| - updateViewportApplyScroll(); |
| - |
| // Plugins can run script inside layout which can detach the page. |
| // TODO(esprehn): Can this still happen now that all plugins are out of |
| // process? |
| @@ -5875,7 +5855,6 @@ DEFINE_TRACE(Document) |
| visitor->trace(m_contextFeatures); |
| visitor->trace(m_styleSheetList); |
| visitor->trace(m_documentTiming); |
| - visitor->trace(m_oldScrollingElement); |
| visitor->trace(m_mediaQueryMatcher); |
| visitor->trace(m_scriptedAnimationController); |
| visitor->trace(m_scriptedIdleTaskController); |
