Incorrect positioning during opacity/transform animation in RTL containers
Reported by
ganor.i...@gmail.com,
Aug 21
|
|||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36 Steps to reproduce the problem: 1. Load the page. Note that during the animation, the div is slightly left-offsetted. When the animation ends, the div returns to its right position. 2. You can click the REFRESH button to see this in action again. 3. You can also change the animation name to transform/opacity, it happens with both of them. What is the expected behavior? The browser should have consider the scrollbar's position, side and width, and position the #container div properly during the animation. What went wrong? Seems like during the animation (of the opacity or the transform properties) in RTL containers, the browser positions #container as if its scrollbar was in the right side of the page. Since it's a RTL page, the scrollbar is on the left side, so things get buggy. Did this work before? N/A Does this work in other browsers? Yes Chrome version: 68.0.3440.106 Channel: stable OS Version: 10.0 Flash Version: My SO thread about it: https://stackoverflow.com/q/51885182 (with an another test case)
,
Aug 21
,
Aug 21
The "snap" doesn't happen on my high-DPI laptop, the content is just always covered by the scrollbar. When I run with --force-device-scale-factor=1 it snaps to the correct location at the end of the animation. This indicates an issue with composited scrollers mis-positioning RTL content. Also of note, this appears to be fixed by --enable-blink-gen-property-trees. Given that's shipping soon I think it's best to wait for that to fix it.
,
Aug 21
The question is how soon is "soon", because it affects all websites in Hebrew, Arabic and other RTL languages. Or maybe there is a workaround that can be implemented with HTML/CSS/JS for the current situation
,
Aug 21
Is this a regression (i.e. is this a new bug that worked correctly in an older version)? The current plan is to ship BGPT in M70 - that's the soonest we'd ship a non-regression fix anyway. If this did regress, we could justify merging to the current beta branch but even that's not sure at this point.
,
Aug 21
From my checkings this happens since Chrome 41. I guess something has changed there that causes this behavior. (I used Browserling to search the oldest version of chrome that have this issue, and it seems to be chrome 41).
,
Aug 23
Able to reproduce issue on reported chrome version 68.0.3497.42 using Ubuntu 17.10. Hence providing bisect information below. Note: The issue is not reproduced on Windows 10 and Mac 10.13.3 Bisect Info: ================ Good build: 61.0.3141.0 Bad build: 61.0.3142.0 CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/7e6f303110c999354c0d343d40e5ac23be3f81d5..aee539101b28c16fc9e38fe648ab0f5d017c4aab Suspect-on: https://chromium.googlesource.com/chromium/src/+/aee539101b28c16fc9e38fe648ab0f5d017c4aab Reviewed-on: https://chromium-review.googlesource.com/538974 vmpstr@: Please confirm the issue and help in re-assigning if it is not related to your change. Thanks!
,
Aug 27
The only reason animations affect this is that they force compositing temporarily, i.e. the content always has the incorrect position on a high DPI device. Removing animation and adding compositing instead.
,
Aug 27
I agree with flackr@, putting will-change: transform on the container persistently positions it slightly off the right edge, by the width of the scrollbar which is now composited. Also, adding overflow: hidden positions it propertly at the right edge with or without compositing, so it's almost certainly the scrollbar being composited that causes this shift. Since this is fixed by BGPT, I would agree with bokan@ that it should be the fix (and it should be coming pretty soon)
,
Aug 27
I hate RTL brokenness like this. I'm happy we should have something shipped in the M70-M71 timeframe. Fixing the underlying issue (not implementing RTL handling in two separate places: blink and cc) is the long-term fix for this and many other related bugs. I don't think we should spend resources for a one-off fix for this sooner (e.g., M69). Lets leave this as P2 and assigned to me for keeping track of it.
,
Aug 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d44db9636279ee5238407ea776dbaab82cfd5363 commit d44db9636279ee5238407ea776dbaab82cfd5363 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Fri Aug 31 23:08:16 2018 [PE] Fix LayoutBox Client/Padding/Content boxes with scrollbars (especially vertical-rl) This CL tries to correct the box model when there are scrollbars, especially in vertical-rl mode. According to https://www.w3.org/TR/css-overflow-3/#scrollbar-layout, scrollbars "should be inserted between the inner border edge and the outer padding edge". Changes to the previous code: - Padding|client box now excludes scrollbars, with the help of (Top|Left|Bottom|Right)ScrollbarWidth methods which can get the scrollbar widths in physical directions in various writing modes. - Content box is now based on the new padding box by excluding the paddings. - Layout of contents is now based on the correct box model. In vertical-rl mode, layout of contents in blocks direction starts from the inner edge of the new content box which has been properly adjusted for the scrollbar. - Now LayoutBox::Location() and Location::PhysicalLocation() in the initial scroll state are correct in all writing-modes. Previously when they were incorrect in vertical-rl mode and some flex box directions, requiring an artificial scroll offset to paint the content at correct place. - With the correct padding box, content box, Location(), PhysicalLocation(), we no longer need the band-aid code to create the correct painted result. The changed code is mostly in LegacyLayout code. Some changed code is in LayoutNG that previously converted correct LayoutNG geometries into the problematic geometries that were previously expected by LegacyLayout. The correct box model is required by blink-gen-property-trees because we can't band-aid the incorrect results in paint properties after painting. Bug: 833167 , 853945 , 858843 , 878809 , 876266 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_layout_ng;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I41faf1ca0bfb95cb287c72703f08c8bd44e9e752 Reviewed-on: https://chromium-review.googlesource.com/1185901 Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#588201} [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/WebKit/LayoutTests/external/wpt/2dcontext/scroll/2d.scrollPathIntoView.verticalRL-expected.txt [add] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/WebKit/LayoutTests/external/wpt/css/cssom-view/cssom-getBoundingClientRect-vertical-rl-ref.html [add] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/WebKit/LayoutTests/external/wpt/css/cssom-view/cssom-getBoundingClientRect-vertical-rl.html [add] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/WebKit/LayoutTests/fast/block/positioning/vertical-rl/absolute-in-vertical-rl-iframe-expected.html [add] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/WebKit/LayoutTests/fast/block/positioning/vertical-rl/absolute-in-vertical-rl-iframe.html [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/BUILD.gn [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/README.md [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_block.h [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_block_flow.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_block_flow_line.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_box.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_box.h [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_box_test.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_flexible_box.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_flexible_box.h [add] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_flexible_box_test.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_object.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_view.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/layout_view.h [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/layout/ng/ng_block_node.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc [modify] https://crrev.com/d44db9636279ee5238407ea776dbaab82cfd5363/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc
,
Aug 31
,
Sep 3
Able to reproduce the issue on build without fix #68.0.3440.106 using Linux 17.10 as per comment #0. Verified the fix on Ubuntu 17.10 as per comment#0 on chrome version #71.0.3541.0. Attaching screen cast for reference. Observed that browser considers the scrollbar's position, side and width, and position the #container div properly during the animation. Adding the verified labels. Thanks...!! |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by e...@chromium.org
, Aug 21Components: -Blink>Layout Blink>Animation Blink>Scroll