[LayoutNG] Fixed positioned objects do not re-position correctly when address bar is hidden |
|||||
Issue descriptionChrome Version: (copy from chrome://version) OS: Android What steps will reproduce the problem? (1) Enable LayoutNG experimental flag. (2) Open https://www.gizmodo.jp/2018/06/amazon-wobeeco-smartwatch.html (3) Scroll down, until the bottom bar appears and until the address bar is hidden. What is the expected result? The bottom blue bar stays at the bottom. What happens instead? The bottom blue bar is up by the height of the address bar.
,
Aug 29
I don't get a bottom bar like that at all when I visit the page without LayoutNG. Maybe it's geo-triggered? In general, the URL bar will affect position: fixed elements but it does not resize the ICB. So if the the bar is |position: fixed, bottom:0| it should stay stuck to the bottom. If it's positioned absolutely/statically then it wont be affected by the URL bar.
,
Aug 29
The page seems to have stopped showing the bottom bar. Found another one: https://japanese.engadget.com/pr/j-score-ai/?ncid=egjp_pr_article_bottom
,
Aug 29
> In general, the URL bar will affect position: fixed elements but it does not resize the ICB. So if the the bar is |position: fixed, bottom:0| it should stay stuck to the bottom. If it's positioned absolutely/statically then it wont be affected by the URL bar. Thanks for the info. Could you mind to tell us, if we're not changing the ICB size, how do we let the |position: fixed, bottom:0| elements to re-position when URL bar is hidden? If it requires additional event, probably LayoutNG is missing it.
,
Aug 29
fixed elements stick to the viewport, which is resized (once the user stops scrolling). During the scroll, the renderer isn't resized while the URL bar moves so we adjust the fixed layers on the compositor but that should be independent of LayoutNG. Once the user lifts their finger, we resize the renderer and Blink (i.e. we get a DOM resize event). AFAIK, fixed elements use the FrameView's frame rect for position which will be resized at this time. IIRC there was code in FrameView relating to this, see LocalFrameView::AddViewportConstrainedObject. I'm not terribly familiar with how those get recalculated but if you follow the calls/reads of the viewport constrained objects it should become clear.
,
Aug 29
and FYI: there's details on how viewport resizing works (it's complicated) here: https://developers.google.com/web/updates/2016/12/url-bar-resizing
,
Sep 18
Thank you so much bokan@, this is a great article. LayoutNG computes ICB from FrameView::GetLayoutSize() and use it everywhere AFAIU. From what you said, I guess we need to use FrameView::FrameRect() instead for fixed elements. Is there any tests where GetLayoutSize() and FrameRect() do not match?
,
Sep 18
The most relevant would be in browser_controls_test.cc, see the 5 tests from and below DontAffectLayoutHeight. If you want to set this up in a test, you'll want to use WebViewImpl::ResizeWithBrowserControls. Set a "top_controls_height" (bottom_controls you can set to 0) and set "browser_controls_shrink_layout" to false. Whatever size you set for "new_size" will be used as the frame_rect size while the ICB will be sized to the new_size - top_controls_height. You can do this now in layout tests too, see LayoutTests/virtual/android/rootscroller/position-fixed-in-unscrollable-document.html.
,
Sep 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3c4c57c94ae1cefa86c0dfb1007998e981de87b6 commit 3c4c57c94ae1cefa86c0dfb1007998e981de87b6 Author: Koji Ishii <kojii@chromium.org> Date: Wed Sep 19 07:52:08 2018 [LayoutNG] unskip virtual/android Unskip "virtual/android" for LayoutNG as these tests seem to be relevant for crbug.com/870008 . This directory was marked to skip in r499723. TBR=mstensho@chromium.org, atotic@chromium.org, cbiesinger@chromium.org NOTRY=true Bug: 870008 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I37094e5174ba1941ae9d927d5462a0985b1ade7a Reviewed-on: https://chromium-review.googlesource.com/1233273 Reviewed-by: Koji Ishii <kojii@chromium.org> Commit-Queue: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#592338} [modify] https://crrev.com/3c4c57c94ae1cefa86c0dfb1007998e981de87b6/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
,
Nov 12
This still bothers my dogfood almost everyday...I'll try if no one is going to pick this up.
,
Nov 12
Will take a look at this today
,
Nov 12
Possibly related are the failing webkit_unit_tests: BrowserControlsSimTest.AffectLayoutHeightWhenConstrained (../../third_party/blink/renderer/core/frame/browser_controls_test.cc:816) BrowserControlsTest.DontAffectLayoutHeight (../../third_party/blink/renderer/core/frame/browser_controls_test.cc:773) (You can now run webkit_unit_test --enable-blink-features=LayoutNG; make sure your tree is up-to-date)
,
Nov 14
Constraint space for position:fixed Elements inside ICB has a special AvailableSize.
Regular element's AvailableSize is "largest possible viewport" size.
position:fixed Available size is the actual size of the viewport.
Example:
<body>
<div id="fix" style="height:100vh;position:fixed"></div>
<div id="abs" style="height:100vh;position:absolute"></div>
#fix.height != #abs.height when url bar is showing.
In the current NG design, sizes needed to generate a fragment
are stored on ConstraintSpace. This will increase CS size
with a record that is almost never used.
@ikilpatrick, you've been playing with minimization of
NGConstraintSpace. Any ideas for an alternative to adding
another rarely used size to NGConstraintSpace?
,
Nov 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68a755f9b2bdc9a682158c61445fb89ba4589c5f commit 68a755f9b2bdc9a682158c61445fb89ba4589c5f Author: Aleks Totic <atotic@chromium.org> Date: Tue Nov 27 08:26:32 2018 [LayoutNG] position:fixed has special ICB There are two tests that test this, but they are currently failing in Legacy too. virtual/android/url-bar/bottom-and-top-fixed-sticks-to-top.html virtual/android/url-bar/bottom-fixed-adjusted-when-showing-url-bar.html I've confirment manually that position:fixed, bottom:0 Elements stick to bottom on scrolling. There is one remaining problem. While scrolling is active, position:fixed, bottom:0 element does not stick to the bottom of the viewport. It gets positioned correctly after scrolling stops. Bug #906516 Bug: 870008 Change-Id: Ibe597141869794978865809caa3991cd4c2bd8a6 Reviewed-on: https://chromium-review.googlesource.com/c/1341127 Commit-Queue: Aleks Totic <atotic@chromium.org> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Reviewed-by: Steve Kobes <skobes@chromium.org> Cr-Commit-Position: refs/heads/master@{#611045} [modify] https://crrev.com/68a755f9b2bdc9a682158c61445fb89ba4589c5f/third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.cc [modify] https://crrev.com/68a755f9b2bdc9a682158c61445fb89ba4589c5f/third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.h [modify] https://crrev.com/68a755f9b2bdc9a682158c61445fb89ba4589c5f/third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.cc [modify] https://crrev.com/68a755f9b2bdc9a682158c61445fb89ba4589c5f/third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.h
,
Dec 3
One small remaining issue tracked in #906516
,
Dec 3
Thank you!
,
Dec 5
This issue is fixed in current M72 build 72.0.3626.7. Verified using the link in the comment #3. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by cbiesin...@chromium.org
, Aug 29