freelifetech.com does not scroll |
|||||||
Issue descriptionChrome Version: 66.0.3350.0 OS: OSX 10.13.3 What steps will reproduce the problem? (1) Open https://freelifetech.com/infrared-remote-control-with-google-home/ (2) Scroll by touch pad, space bar, page down key, etc. What is the expected result? Scroll. What happens instead? Does not scroll.
,
Feb 22 2018
kojii@ This seems to scroll just fine for me. Do you have any feature flags on?
,
Feb 23 2018
Yeah, right, I had: --enable-experimental-web-platform-features When I disabled it, it scrolled fine for me too. Sorry for the noise.
,
Feb 23 2018
It looks like this is caused by ImplicitRootScroller.
,
Mar 21 2018
,
Apr 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eebeab8487b2fb74cf048e5e24cf5678856cf44e commit eebeab8487b2fb74cf048e5e24cf5678856cf44e Author: David Bokan <bokan@chromium.org> Date: Tue Apr 24 21:58:33 2018 Scroll documentElement using LayoutView natively When computing the scroll-chain for a gesture, only Elements can be added to the chain. However, the document element (i.e. <html>) doesn't have a ScrollableArea itself. Its scrolling is delegated to the document *node* (i.e. the LayoutView) which cannot be added to the chain. Thus, we use the documentElement to represent scrolling the LayoutView. See ScrollManager::RecomputeScrollChain to see this explicitly in code. When actually performing the scroll we need to reverse the above substitution but we currently have a bug: in Element::NativeApplyScroll we apply this substitution if we're scrolling the effective root scroller rather than the document element. If the root scroller API is enabled and the root scroller is changed to a different element, the document can no longer be scrolled since it will fail to reverse the documentElement<->LayoutView substitution and try to use the document element's ScrollableArea which doesn't exist. A similar fix is needed in LogicalScroll for keyboard scrolls. This complicated logic is encoded in the scroll-chain calculation so I've rewritten that method to use the scroll chain for consistency. An additional change is needed since LayoutView::Scroll uses LocalFrameView::GetScrollableArea to scroll. In the main frame, this will be the RootFrameViewport which uses the global root scroller as the layout viewport. Thus, if the root scroller is set to another element, scrolling the LayoutView would actually scroll that element. The global root scroller will always be scrolled using the correct ScrollableArea via the ViewportScrollCallback so LayoutView shouldn't be treated specially in this case. In fact, I've removed LayoutBox::Scroll and its overrides altogether and replaced its call sites with ScrollableArea directly. Bug: 813747 Change-Id: I9e9a27ff684abf969c27ac152c860778a7e00e39 Reviewed-on: https://chromium-review.googlesource.com/1018880 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#553303} [add] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/WebKit/LayoutTests/rootscroller/gesture-scroll-document-not-root-scroller.html [add] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/WebKit/LayoutTests/rootscroller/keyboard-scroll-document-not-root-scroller.html [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/dom/element.cc [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/exported/web_frame_test.cc [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/exported/web_view_test.cc [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/input/scroll_manager.cc [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/layout/layout_box.cc [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/layout/layout_box.h [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/layout/layout_embedded_object.cc [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/layout/layout_embedded_object.h [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/layout/layout_view.cc [modify] https://crrev.com/eebeab8487b2fb74cf048e5e24cf5678856cf44e/third_party/blink/renderer/core/layout/layout_view.h
,
Apr 24 2018
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/936338b61aab7784912216f8e7881498805f1025 commit 936338b61aab7784912216f8e7881498805f1025 Author: Max Morin <maxmorin@chromium.org> Date: Thu Apr 26 11:23:07 2018 Revert "Scroll documentElement using LayoutView natively" This reverts commit eebeab8487b2fb74cf048e5e24cf5678856cf44e. Reason for revert: Reverting due to failing test ( crbug.com/836623 ). Original change's description: > Scroll documentElement using LayoutView natively > > When computing the scroll-chain for a gesture, only Elements can be > added to the chain. However, the document element (i.e. <html>) doesn't > have a ScrollableArea itself. Its scrolling is delegated to the > document *node* (i.e. the LayoutView) which cannot be added to the > chain. Thus, we use the documentElement to represent scrolling the > LayoutView. See ScrollManager::RecomputeScrollChain to see this > explicitly in code. > > When actually performing the scroll we need to reverse the above > substitution but we currently have a bug: in Element::NativeApplyScroll > we apply this substitution if we're scrolling the effective root > scroller rather than the document element. If the root scroller API is > enabled and the root scroller is changed to a different element, the > document can no longer be scrolled since it will fail to reverse the > documentElement<->LayoutView substitution and try to use the document > element's ScrollableArea which doesn't exist. > > A similar fix is needed in LogicalScroll for keyboard scrolls. This > complicated logic is encoded in the scroll-chain calculation so I've > rewritten that method to use the scroll chain for consistency. > > An additional change is needed since LayoutView::Scroll uses > LocalFrameView::GetScrollableArea to scroll. In the main frame, this > will be the RootFrameViewport which uses the global root scroller as > the layout viewport. Thus, if the root scroller is set to another > element, scrolling the LayoutView would actually scroll that element. > The global root scroller will always be scrolled using the correct > ScrollableArea via the ViewportScrollCallback so LayoutView shouldn't > be treated specially in this case. In fact, I've removed > LayoutBox::Scroll and its overrides altogether and replaced its > call sites with ScrollableArea directly. > > Bug: 813747 > Change-Id: I9e9a27ff684abf969c27ac152c860778a7e00e39 > Reviewed-on: https://chromium-review.googlesource.com/1018880 > Reviewed-by: Steve Kobes <skobes@chromium.org> > Commit-Queue: David Bokan <bokan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#553303} TBR=bokan@chromium.org,skobes@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. No-Try: true Bug: 813747 , 836623 Change-Id: Iaa92fdb566bec60fcdc2f03e9990fc4aacab0be1 Reviewed-on: https://chromium-review.googlesource.com/1029971 Commit-Queue: Max Morin <maxmorin@chromium.org> Reviewed-by: Max Morin <maxmorin@chromium.org> Cr-Commit-Position: refs/heads/master@{#553981} [delete] https://crrev.com/83be57d40f5c53b09fe79186d1e01eb608759a62/third_party/WebKit/LayoutTests/rootscroller/gesture-scroll-document-not-root-scroller.html [delete] https://crrev.com/83be57d40f5c53b09fe79186d1e01eb608759a62/third_party/WebKit/LayoutTests/rootscroller/keyboard-scroll-document-not-root-scroller.html [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/dom/element.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/exported/web_frame_test.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/exported/web_view_test.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/input/scroll_manager.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_box.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_box.h [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_embedded_object.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_embedded_object.h [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_view.cc [modify] https://crrev.com/936338b61aab7784912216f8e7881498805f1025/third_party/blink/renderer/core/layout/layout_view.h
,
Apr 26 2018
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d9829873bac0d85ef9c4bbe26999b3f989e37ba5 commit d9829873bac0d85ef9c4bbe26999b3f989e37ba5 Author: David Bokan <bokan@chromium.org> Date: Thu Apr 26 16:37:15 2018 [Reland] Scroll documentElement using LayoutView natively When computing the scroll-chain for a gesture, only Elements can be added to the chain. However, the document element (i.e. <html>) doesn't have a ScrollableArea itself. Its scrolling is delegated to the document *node* (i.e. the LayoutView) which cannot be added to the chain. Thus, we use the documentElement to represent scrolling the LayoutView. See ScrollManager::RecomputeScrollChain to see this explicitly in code. When actually performing the scroll we need to reverse the above substitution but we currently have a bug: in Element::NativeApplyScroll we apply this substitution if we're scrolling the effective root scroller rather than the document element. If the root scroller API is enabled and the root scroller is changed to a different element, the document can no longer be scrolled since it will fail to reverse the documentElement<->LayoutView substitution and try to use the document element's ScrollableArea which doesn't exist. A similar fix is needed in LogicalScroll for keyboard scrolls. This complicated logic is encoded in the scroll-chain calculation so I've rewritten that method to use the scroll chain for consistency. An additional change is needed since LayoutView::Scroll uses LocalFrameView::GetScrollableArea to scroll. In the main frame, this will be the RootFrameViewport which uses the global root scroller as the layout viewport. Thus, if the root scroller is set to another element, scrolling the LayoutView would actually scroll that element. The global root scroller will always be scrolled using the correct ScrollableArea via the ViewportScrollCallback so LayoutView shouldn't be treated specially in this case. In fact, I've removed LayoutBox::Scroll and its overrides altogether and replaced its call sites with ScrollableArea directly. Reland Note: Typo in test caused incorrect async call chain in promises. Previously landed patch is PS1 for easy comparison. TBR=skobes@chromium.org Bug: 813747 , 836623 Change-Id: Ie46e6f9142b4a4d815cb1e3cb8a99a338f96796e Reviewed-on: https://chromium-review.googlesource.com/1030055 Reviewed-by: David Bokan <bokan@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#554049} [add] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/WebKit/LayoutTests/rootscroller/gesture-scroll-document-not-root-scroller.html [add] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/WebKit/LayoutTests/rootscroller/keyboard-scroll-document-not-root-scroller.html [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/dom/element.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/exported/web_frame_test.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/exported/web_view_test.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/input/scroll_manager.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_box.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_box.h [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_embedded_object.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_embedded_object.h [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_view.cc [modify] https://crrev.com/d9829873bac0d85ef9c4bbe26999b3f989e37ba5/third_party/blink/renderer/core/layout/layout_view.h
,
Apr 26 2018
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by kojii@chromium.org
, Feb 20 2018