[RLS] LocalFrameView::ScrollOrigin() should be 0,0 |
||
Issue descriptionLog scroll origin in LocalFrameView::AdjustViewSize and run: content_shell --root-layer-scrolls https://output.jsbin.com/pabuxig/quiet It shows 848,0, then 863,0. It should never be anything except 0,0 - the document's negative horizontal overflow should be applied to the layout viewport only.
,
Aug 22 2017
Logs for RLS/Non-RLS case. ************************ RLS ************************* [1:1:0822/195644.230778:3401772401937:ERROR:LocalFrameView.cpp(757)] origin: 848, 0 [1:1:0822/195644.232002:3401772403130:ERROR:LocalFrameView.cpp(757)] origin: 863, 0 ************************************************ *********************** Non RLS ********************* [1:1:0822/195802.643476:3401850814687:ERROR:LocalFrameView.cpp(757)] origin: 863, 0 [1:1:0822/195802.644224:3401850815374:ERROR:LocalFrameView.cpp(757)] origin: 863, 0 ****************************************************
,
Aug 23 2017
The only difference between RLS/NonRLS case is the first value 848 instead of 863. So should we check that alone or should we check why 863 is coming instead of 0, in NonRLS case as well?
,
Aug 23 2017
Without RLS, the scroll origin should be applied to the LocalFrameView, so 863 is correct. With RLS, the scroll origin should be applied to the root PaintLayerScrollableArea instead of the LocalFrameView. So the root PLSA should have a scroll origin of 863, and the LocalFrameView should have a scroll origin of 0. Probably it is sufficient for LocalFrameView::AdjustViewSize to check the RLS setting and skip the call to SetScrollOrigin. Currently it sets scroll origin based on LayoutView::DocumentRect which is the LayoutView's layout overflow rect.
,
Aug 23 2017
I think the change from 848 to 863 is due to the creation of the root PLSA's vertical scrollbar. There's a separate bug to start with a vertical scrollbar by default ( issue 711137 ).
,
Aug 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e26a0a9d9246ab486707d49cd17a27688e239961 commit e26a0a9d9246ab486707d49cd17a27688e239961 Author: srirama <srirama.m@samsung.com> Date: Thu Aug 24 18:01:45 2017 [RLS] LocalFrameView::ScrollOrigin() should be 0,0 The document's negative horizontal overflow should be applied to the layout viewport only. Currently it sets scroll origin based on LayoutView::DocumentRect which is the LayoutView's layout overflow rect. Incase of RLS it should be applied to the root PaintLayerScrollableArea only. Added RLS enabled check to avoid wrong updation of scroll origin. BUG= 757636 Change-Id: I9e0f7ea31b71da7dcb81a40a661a45d5a150d65c Reviewed-on: https://chromium-review.googlesource.com/628858 Reviewed-by: Steve Kobes <skobes@chromium.org> Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com> Cr-Commit-Position: refs/heads/master@{#497120} [modify] https://crrev.com/e26a0a9d9246ab486707d49cd17a27688e239961/third_party/WebKit/Source/core/exported/WebFrameTest.cpp [modify] https://crrev.com/e26a0a9d9246ab486707d49cd17a27688e239961/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
,
Aug 24 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by skobes@chromium.org
, Aug 21 2017