Status Update
Comments
ik...@chromium.org <ik...@chromium.org> #2
Thank you for the PoC and video. chrishtr@, can you take a look at this? Please feel free to re-route if necessary.
ik...@chromium.org <ik...@chromium.org> #3
sh...@chromium.org <sh...@chromium.org> #4
sc...@chromium.org <sc...@chromium.org> #5
rb...@chromium.org <rb...@chromium.org> #6
Thanks for taking a look at this. AFAIK it should be an issue with the Intersection Observer v2, as in some cases, it fails to detect that the observing element is not visible.
Please try following to reproduce the issue:
- Create a new profile or clear cache and site data
- Go to
https://pocs.work/tools/framer/#https://pocs.work/demo/intersection-observer-v2/frame.html - You should see an iframe with a red background, meaning that the Observer v2 detected that it's not visible -> correct behavior
- Go to
https://pocs.lol/tools/framer/#https://pocs.work/demo/intersection-observer-v2/frame.html - You should see an iframe with a green background, meaning that the Observer v2 failed to detect that it's not visible -> bug
IMHO, it's some sort of caching issue, since after refreshing several times, it's sometimes working as expected again. I tested it on macOS and Linux.
The Google SSO just makes use of this API to prevent clickjacking, but since it does not work reliably, clickjacking attacks are possible (see other poc video).
I attached another PoC video that shows the unexpected Observer v2 API behavior. Note that this PoC uses the same JavaScript implementation as the
bu...@chromium.org <bu...@chromium.org> #7
Here's a PoC video with the latest version of Chromium on Ubuntu 22.04.3 LTS.
su...@chromium.org <su...@chromium.org> #8
Also, it seems that as soon as the window is resized (or scrolled), the Observer v2 starts to refresh (recalculate its visibility) and then detects that it is not visible anymore, see the attached poc video.
sh...@chromium.org <sh...@chromium.org> #9
If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?
If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.
Thanks for your time! To disable nags, add Disable-Nags (case sensitive) to the Chromium Labels custom field.
sc...@chromium.org <sc...@chromium.org> #10
Branch: main
commit 78bd1f0eef2612f22dac87d0335e8714a0ecee89
Author: Stefan Zager <szager@chromium.org>
Date: Thu May 16 05:52:33 2024
[IntersectionObservation] Fix needs_occlusion_tracking bookkeeping
This fixes three related bugs in occlusion tracking:
1. When an IntersectionObserver using trackVisibility is created in an
OOPIF, there is some latency in notifying the parent renderer that
occlusion tracking is required for the iframe. During that interval,
the parent renderer might send a 'guaranteed not occluded' message to
the child, causing the IntersectionObserver to report a false
positive. With this change, we will never tell an OOPIF that it's
"guaranteed not occluded" if the OOPIF hasn't requested occlusion
information. This is fixed in frame_view.cc.
2. When an OOPIF runs LocalFrameView::RunIntersectionObserverSteps,
it might get a transient `false` value when recomputing
needs_occlusion_tracking, due to an early return from
UpdateViewportIntersectionsForSubtree (which has a couple of
early-return edge cases). When that happens, we should *not* relay
the `false` value back to the parent process, because it can create
a temporary gap in occlusion reporting from the parent. We should
instead preserve the previous value until it can be successfully
recomputed on a subsequent BeginMainFrame. This is fixed in
local_frame_view.cc.
2. For OOPIF's, needs_occlusion_tracking is stored in two places:
on RemoteFrameOwner::needs_occlusion_tracking_ in the iframe process,
and in RemoteFrameView::needs_occlusion_tracking_ in the parent
process. When the first one changes in the iframe process, it IPC's
to the parent process to synchronize the second one. To keep them
consistent, there should be no other point of entry into
RemoteFrameView::SetNeedsOcclusionTracking. Currently, that method is
also called from RemoteFrameView::Dispose, but that call is unnecessary
and it can cause the two values to get out of sync. This is fixed in
remote_frame_view.cc.
Bug: chromium:333708039
Change-Id: Id1dd08eefb5aa201651b2c6950ea61f3b4a081a9
Reviewed-on:
Commit-Queue: Stefan Zager <szager@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1301762}
M third_party/blink/renderer/core/frame/frame_view.cc
M third_party/blink/renderer/core/frame/local_frame_view.cc
M third_party/blink/renderer/core/frame/remote_frame_view.cc
M third_party/blink/web_tests/TestExpectations
ea...@chromium.org <ea...@chromium.org> #11
sh...@chromium.org <sh...@chromium.org> #12
Thanks for resolving the vulnerability. I can confirm that it is fixed in 127.0.6485.0 (Official Build) dev (arm64).
ea...@chromium.org <ea...@chromium.org> #13
If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?
If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.
Thanks for your time! To disable nags, add Disable-Nags (case sensitive) to the Chromium Labels custom field.
hi...@jonjohnjohnson.com <hi...@jonjohnjohnson.com> #14
Hello,
Congratulations! The Chrome Vulnerability Rewards Program (VRP) Panel has decided to award you $5000.00 for this report.
Rationale for this decision:
$5,000 for high quality report (with functional exploit) of user information disclosure
Important: If you aren't already registered with Google as a supplier, p2p-vrp@google.com will reach out to you. If you have registered in the past, no need to repeat the process – you can sit back and relax, and we will process the payment soon.
If you have any payment related requests, please direct them to p2p-vrp@google.com. Please remember to include the subject of this email and the email address that the report was sent from.
Thank you for your efforts and helping us make Chrome more secure for all users!
Cheers,
Chrome VRP Panel Bot
P.S. One other thing we'd like to mention:
* Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
Please contact security-vrp@chromium.org with any questions.
ea...@chromium.org <ea...@chromium.org> #15
Congratulations Louis! Thank you for your efforts and reporting this issue to us. In the future (which we hope you'll report bugs to us in :) please kindly upload the POC directly to the report and avoid linking it. We do not want to encourage or provide the opportunity for engineers or others to click arbitrary links to potentially malicious web content. Separate POC files should be included so that can easily scp that file to our contained VMs for reproduction and investigation. Thank you and nice work!
ms...@chromium.org <ms...@chromium.org> #16
Awesome, thanks for the reward! Alright, I will upload the PoC from now on.
ms...@chromium.org <ms...@chromium.org> #17
This bug has been closed for more than 14 weeks. Removing issue access restrictions.
bu...@chops-service-accounts.iam.gserviceaccount.com <bu...@chops-service-accounts.iam.gserviceaccount.com> #18
commit 12ab8499f9e2519b4a448b5f39da05ea42bb539f
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Jun 25 14:50:30 2019
Correct bug numbers for incoming multicol test failures.
TBR=eae@chromium.org
Bug: 481431, 978371
Change-Id: If5355445e48d3b022b9194b1efc0c8a423d55076
Reviewed-on:
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672075}
[modify]
ms...@chromium.org <ms...@chromium.org> #19
ms...@chromium.org <ms...@chromium.org> #20
ms...@chromium.org <ms...@chromium.org> #22
gi...@appspot.gserviceaccount.com <gi...@appspot.gserviceaccount.com> #23
commit 746885d1ca2be9d481741a4be76c9ffb1adc1bcf
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Feb 15 11:21:36 2022
Use designated bug for box-decoration-break:clone.
Bug: 481431, 1296860
Change-Id: Ia8e319f72f008ba12f95931517c0e43e85549a05
Reviewed-on:
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#971155}
[modify]
ms...@chromium.org <ms...@chromium.org> #24
gi...@appspot.gserviceaccount.com <gi...@appspot.gserviceaccount.com> #25
commit d08cf9910986993926f994fb3f599916f41a3247
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Mon Feb 21 09:27:19 2022
Associate multicol-zero-height-003.html failure with the right bug.
external/wpt/css/css-multicol/multicol-zero-height-003.html fails
because we don't support box-decoration-break:clone
Bug: 481431, 1299230
Change-Id: Ia725e3dff9009782fdeceacac56319bbebe59d9b
Reviewed-on:
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#973460}
[modify]
ms...@chromium.org <ms...@chromium.org> #26
See
Changing dependencies (NG printing).
Hopefully we can add support for this soon.
ms...@chromium.org <ms...@chromium.org> #27
ms...@chromium.org <ms...@chromium.org> #28
ms...@chromium.org <ms...@chromium.org> #29
is...@google.com <is...@google.com> #30
[Monorail blocking:
[Monorail mergedwith:
[Monorail components added to Component Tags custom field.]
ms...@chromium.org <ms...@chromium.org>
ap...@google.com <ap...@google.com> #31
Branch: main
commit 2097a70d9cdce8e43e6d3ebec9e0d15baca19ccb
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Fri Jul 12 08:24:17 2024
Pass fragmentainer capacity to fragmentation util functions.
The idea here is that the constraint space won't always know the correct
amount of space available, if box decorations are to be cloned (yet to
be landed). Although the fragment builder will know the size, the
fragment builder isn't always available (e.g. with floats laid out from
the inline layout algorithm) at every call site. And this is a good
thing, since we sometimes want to consult the fragmentation machinery
without any side-effects.
No behavior changes intended.
Bug: 40415661
Change-Id: Ieba4c34f14b3d519b9bdf73d725b0c6d87832e67
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1326620}
M third_party/blink/renderer/core/layout/block_layout_algorithm.cc
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.cc
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.h
M third_party/blink/renderer/core/layout/floats_utils.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.h
M third_party/blink/renderer/core/layout/grid/grid_layout_algorithm.cc
M third_party/blink/renderer/core/layout/layout_algorithm.h
M third_party/blink/renderer/core/layout/out_of_flow_layout_part.cc
ap...@google.com <ap...@google.com> #32
Branch: main
commit 70be3a608b6c394325e56cf4a45f48d2428f4b8d
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Fri Jul 12 09:32:31 2024
Pass fragment builder when calculating fragmentainer block-size.
The constraint space won't always know the full story in cases where box
decorations are to be cloned. Cloned box decorations (yet to be landed)
will effectively shrink the fragmentainer space available to children.
Also add another convenience function for FragmentainerSpaceLeft() to
LayoutAlgorithm, rather than having to pass the builder manually every
time.
The builder isn't available when laying out floats, so that
UnpositionedFloat instead need to carry information about fragmentainer
size.
No behavior changes intended.
Bug: 40415661
Change-Id: I5eef5a51b7c123a9899ea4959ba3f0299c0e9016
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1326644}
M third_party/blink/renderer/core/layout/block_layout_algorithm.cc
M third_party/blink/renderer/core/layout/box_fragment_builder.cc
M third_party/blink/renderer/core/layout/column_layout_algorithm.cc
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.cc
M third_party/blink/renderer/core/layout/floats_utils.cc
M third_party/blink/renderer/core/layout/forms/fieldset_layout_algorithm.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.h
M third_party/blink/renderer/core/layout/grid/grid_layout_algorithm.cc
M third_party/blink/renderer/core/layout/inline/inline_layout_algorithm.cc
M third_party/blink/renderer/core/layout/inline/line_breaker.cc
M third_party/blink/renderer/core/layout/layout_algorithm.h
M third_party/blink/renderer/core/layout/out_of_flow_layout_part.cc
M third_party/blink/renderer/core/layout/table/table_layout_algorithm.cc
M third_party/blink/renderer/core/layout/unpositioned_float.h
ap...@google.com <ap...@google.com> #33
Branch: main
commit 5f4915c32412099313a3451f29ee867e6e27d62b
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Fri Jul 12 10:29:19 2024
Support for cloned box decorations inside block fragmentation.
There's some remaining work for flex and grid layout, and nested
multicol. This CL only fixes it for block containers and tables. There
happens to be some tests for tables already [1], and we don't want to
regress them (the refs were also using box-decoration-break:clone, so
they actually pass with no box decoration cloning support at all, but
would fail with incorrect box decoration cloning support).
BorderPadding() of a fragment builder now include border+padding from
previous fragments, so that ComputeBlockSizeForFragment() does the right
thing.
BorderScrollbarPadding() is for positioning and sizing things inside the
current fragment only, had has traditionally unconditionally removed the
block-start part after fragmentation breaks. Don't do this if box
decorations are to be cloned.
Also note that the "scrollbar" part of BorderScrollbarPadding() is
always 0 when block fragmentation is involved, since scrollable elements
are always monolithic.
The remaining work for flex, grid, and nested multicol is mainly about
using BorderPadding() vs BorderScrollbarPadding() correctly, and also
not assume that IsBreakInside(incoming_break_token) implies that the
start block-offset for children should be 0.
[1] e.g. css/css-break/table/table-fragmentation-001c-print.html
Bug: 40415661
Change-Id: If198b90d4ee0501d0ca66c909bbed1be37c7c5b8
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1326658}
M third_party/blink/renderer/core/layout/absolute_utils.cc
M third_party/blink/renderer/core/layout/box_fragment_builder.cc
M third_party/blink/renderer/core/layout/box_fragment_builder.h
M third_party/blink/renderer/core/layout/floats_utils.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.h
M third_party/blink/renderer/core/layout/out_of_flow_layout_part.cc
M third_party/blink/renderer/core/layout/table/table_layout_algorithm.cc
M third_party/blink/web_tests/TestExpectations
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-005.tentative.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-006.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-007.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-008.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-009-ref.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-009.html
ap...@google.com <ap...@google.com> #34
Branch: main
commit 31e6127ba96ee31daef1734f8fff1d1b16d092e7
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Mon Jul 15 17:36:20 2024
Multicol: Don't pass accumulated decorations to FinishFragmentation.
BorderPadding() is for the node as a whole, and will include cloned box
decorations from previous fragments. Use BorderScrollbarPadding()
instead, which is just for the current fragment.
Note: The block-end box decorations don't really play any role in
FinishFragmentation() if box decorations are cloned, but there's a
DCHECK that it's not larger than the fragment. The test included would
fail this DCHECK, but pass apart from that.
In general, cloned box decorations is already working fine for nested
multicol, and there is an existing test at
css/css-multicol/multicol-breaking-004.html
Bug: 40415661
Change-Id: I80af587887ea6e45c50edccdcbfc211492176886
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1327601}
M third_party/blink/renderer/core/layout/column_layout_algorithm.cc
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-010.html
ap...@google.com <ap...@google.com> #35
Branch: main
commit 59f08f800a363c32e086da43022cd9469fc84f48
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Jul 16 21:26:40 2024
Cleanup for decoration cloning awareness in fragmentation utils.
Replace the include_cloned_block_end_decorations parameter with
is_for_children (and reverse logic), and don't provide a default value,
as this was very bug prone: Added TODOs for places that had gotten it
wrong, except in FinishFragmentationForFragmentainer(): Updated
FragmentainerCapacity() calls there right away to pass
is_for_children=false, which is the opposite of before, but that won't
matter, since fragmentainers (currently) cannot have box decorations.
In LayoutAlgorithm, rename FragmentainerCapacity() and
FragmentainerSpaceLeft() to FragmentainerCapacityForChildren() and
FragmentainerSpaceLeftForChildren().
No behavior changes intended.
Bug: 40415661
Change-Id: Ib650682da1710c7ed48e6fc6fee099cadb02f7b3
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1328440}
M third_party/blink/renderer/core/layout/block_layout_algorithm.cc
M third_party/blink/renderer/core/layout/box_fragment_builder.cc
M third_party/blink/renderer/core/layout/column_layout_algorithm.cc
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.cc
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.h
M third_party/blink/renderer/core/layout/forms/fieldset_layout_algorithm.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.h
M third_party/blink/renderer/core/layout/grid/grid_layout_algorithm.cc
M third_party/blink/renderer/core/layout/layout_algorithm.h
M third_party/blink/renderer/core/layout/table/table_layout_algorithm.cc
ap...@google.com <ap...@google.com> #36
Branch: main
commit b5d1b2fb8c8be9400510ecd99ba88163f70b58f0
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Jul 17 10:00:35 2024
Prevent break between cloned box decoration and 1st child fragment.
This would cause an infinite loop, due to a last-resort break being
inserted before a tall and unbreakable first-child-fragment over and
over again. Last-resort breaks are questionable in general, but in the
case of cloned box decorations, there's really no question: They must be
avoided! Avoid this by adjusting the fragmentainer block-offset more
consistently when cloned decorations are used. We previously did this
for child constraint spaces, but the fragment builder (the parent of
such constraint spaces) that establishes cloned decorations really also
needs to do this.
Add blink::FragmentationOffset() that takes a builder argument, rather
than using ConstraintSpace::FragmentainerOffset() directly, so that we
can pay attention to cloned block-start decorations. Also add a
FragmentationOffsetForChildren() convenience function to
LayoutAlgorithm.
This is all pretty simple, except when it comes from floats, which has
no access to the parent fragment builder (they don't even know if it's
an inline or block layout algorithm - and they really don't want to
know, either). UnpositionedFloat now needs to know the fragmentainer
offset (it already knew the fragmentainer block size). The fragmentainer
offset is meaningless unless inside block fragmentation, and there even
used to be a DCHECK in ConstraintSpace::FragmentainerOffset() for it.
Just remove it, rather than worrying about this at every call site.
Also flip is_for_children to false in call to FragmentainerSpaceLeft()
in SetupFragmentBuilderForFragmentation(). This is correct, although it
doesn't matter much with the way the code is right now, since we haven't
yet told the builder whether block-end decorations should be cloned at
this point. There are tests for this situation (when the remainder of
the node fits in the current fragmentainer, and block-end decorations
should therefore not be cloned, but behave as regular block-end
decorations), such as css/css-break/box-decoration-break-clone-002.html
Bug: 40415661
Change-Id: I2f1d596fa638435b5f6f66dd5ae234fb48e26475
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1328733}
M third_party/blink/renderer/core/layout/block_layout_algorithm.cc
M third_party/blink/renderer/core/layout/column_layout_algorithm.cc
M third_party/blink/renderer/core/layout/constraint_space.h
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.cc
M third_party/blink/renderer/core/layout/floats_utils.cc
M third_party/blink/renderer/core/layout/forms/fieldset_layout_algorithm.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.h
M third_party/blink/renderer/core/layout/inline/inline_layout_algorithm.cc
M third_party/blink/renderer/core/layout/inline/line_breaker.cc
M third_party/blink/renderer/core/layout/layout_algorithm.h
M third_party/blink/renderer/core/layout/out_of_flow_layout_part.cc
M third_party/blink/renderer/core/layout/table/table_layout_algorithm.cc
M third_party/blink/renderer/core/layout/table/table_section_layout_algorithm.cc
M third_party/blink/renderer/core/layout/unpositioned_float.h
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-011.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-012.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-013.html
ap...@google.com <ap...@google.com> #37
Branch: main
commit 5841a8c6f28b2c10956cd9a153eb4a535154d3e2
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Jul 17 10:17:05 2024
Better box decoration cloning around monolithic content.
Not a very important thing, but I discovered and fixed some
imperfections in the code while looking into this, which should be good
for health.
The correct thing to do for tall overflowing monolithic content: don't
clone box decorations of the container, since the container itself will
also become equally tall and monolithic in such cases.
In order for FinishFragmentation() to correctly understand when we're
past the block-end of a node, update space_left if there's monolithic
content inside forcing the node itself to use more space as well.
There's no spec to properly guide us here, but it should at least be
better now.
While working on this, I discovered that the overflow calculation code
in PhysicalBoxFragment::Create() got the wrong border / padding values -
i.e. any value that should be truncated wasn't. Fixing this also
required fixing the LayoutTableColumn implementation of offsetHeight &
co, since it was assuming that every fragment had the box decoration
stored, even when they were not part of the fragment. For now, only
truncate along the block axis. If we could also do the same to inline
decorations, we could probably remove SidesToInclude() from
PhysicalBoxFragment.
There's some fairly good inter-op with Gecko here, but one of the new
tests is failing in Blink (but I believe it's correct, and it should be
passing in Gecko).
Bug: 40415661
Change-Id: I95e8468c071d0a97aa12fc3823b26b17b02a7af6
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1328742}
M third_party/blink/renderer/core/layout/box_fragment_builder.cc
M third_party/blink/renderer/core/layout/box_fragment_builder.h
M third_party/blink/renderer/core/layout/fragmentation_utils.cc
M third_party/blink/renderer/core/layout/physical_box_fragment.cc
M third_party/blink/renderer/core/layout/table/layout_table_column.cc
M third_party/blink/web_tests/TestExpectations
A third_party/blink/web_tests/external/wpt/css/css-page/monolithic-overflow-031-print-ref.html
A third_party/blink/web_tests/external/wpt/css/css-page/monolithic-overflow-031-print.html
A third_party/blink/web_tests/external/wpt/css/css-page/monolithic-overflow-032-print-ref.html
A third_party/blink/web_tests/external/wpt/css/css-page/monolithic-overflow-032-print.tentative.html
A third_party/blink/web_tests/wpt_internal/printing/monolithic-overflow-001-print-ref.html
A third_party/blink/web_tests/wpt_internal/printing/monolithic-overflow-001-print.html
ap...@google.com <ap...@google.com> #38
Branch: main
commit 85787b76da603172ffaa3add1153494dda33c153
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Thu Jul 18 19:10:41 2024
Handle cloned box decorations in flex containers.
Don't assume that items resume at block-offset in a flex container
fragment after a break. There may be cloned box decorations there. Just
use BorderScrollbarPadding(). If box decorations are to be sliced, they
will be 0 there.
Also only use BorderPadding() when resolving the total node size, since
cloned box decorations are multiplied there. Use
BorderScrollbarPadding() in call to FinishFragmentation().
Items in flex containers are distributed before fragmentation, and then
adjusted afterwards. If fragmentation inserts cloned box decorations on
the flex container, we need to compensate for that when dealing with
values based on unfragmented layout. We need something almost like
"previously consumed block-size", but not quite. Previously consumed
block-size of a flex container includes any cloned box decorations, but
they are not part of the imaginary unfragmented flex container. This is
what offset_in_stitched_container is about.
GiveItemsFinalPositionAndSizeForFragmentation() will now use that one
instead of previously_consumed_block_size in almost all cases, except
those that are about sizing the container itself.
Since cloned box decorations effectively grows the border box size of a
node each time it fragments, the block-size of items in a flex column
flow needed s\some adjustments. Since the flex algorithm is responsible
for calculating the block-size of an item, it is fixed/locked for the
child layout algorithm, which means that flex items (unlike children in
a regular block container) may not resolve their own block-size. We
therefore need to adjust it every time the item breaks.
Bug: 40415661
Change-Id: I604b937898db75e0b0d86731410562f848ed1307
Reviewed-on:
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1329733}
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.cc
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.h
M third_party/blink/renderer/core/layout/fragmentation_utils.h
M third_party/blink/renderer/core/layout/logical_box_fragment.h
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-014.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-015.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-016.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-017.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-018.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-019.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-020.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-021.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-022.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-023.html
ap...@google.com <ap...@google.com> #39
Branch: main
commit d2bc471d08a213e38a903049266b72da2e287570
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Jul 24 10:51:59 2024
Clean up box decoration adjustment in flex fragmentation.
Follow-up to
No behavior changes intended.
Bug: 40415661
Change-Id: I7d9f7c9bd9d2977b255838d2a68aa70e2f887930
Reviewed-on:
Reviewed-by: Alison Maher <almaher@microsoft.com>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1332244}
M third_party/blink/renderer/core/layout/flex/flex_layout_algorithm.cc
ap...@google.com <ap...@google.com> #40
Branch: main
commit f2955d6b26f484d5f0a88b3bb4434e319d3ce881
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Fri Aug 09 21:33:50 2024
Handle cloned box decorations in fieldsets.
Bug: 40415661
Change-Id: I949bfc6e992be031daf776cace34775f875bb0ef
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1339882}
M third_party/blink/renderer/core/layout/box_fragment_builder.h
M third_party/blink/renderer/core/layout/forms/fieldset_layout_algorithm.cc
M third_party/blink/renderer/core/layout/forms/layout_fieldset.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.cc
M third_party/blink/renderer/core/layout/fragmentation_utils.h
M third_party/blink/renderer/core/layout/layout_algorithm.h
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-024.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-025.html
ap...@google.com <ap...@google.com> #41
Branch: main
commit 0a0a5e0b37c07019222285fea36da3af6a70e82a
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Aug 20 16:13:08 2024
Add test for cloned border-radius.
This has already been working for some time, but there was no test
coverage.
Bug: 40415661
Change-Id: I5e172747d278e66019414285e99b2c9b3578334d
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1344190}
A third_party/blink/web_tests/external/wpt/css/css-break/borders-008-ref.html
A third_party/blink/web_tests/external/wpt/css/css-break/borders-008.html
ap...@google.com <ap...@google.com> #42
Branch: main
commit 667c1944909073156eb17487998ea026fd280425
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Aug 20 16:12:24 2024
Add test for cloned box-shadow.
This has already been working for some time, but there was no test
coverage.
Bug: 40415661
Change-Id: I796533abc7507cfe69ffae3cc9a58a5ba3f29eb3
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1344189}
A third_party/blink/web_tests/external/wpt/css/css-break/box-shadow-005-ref.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-shadow-005.html
ap...@google.com <ap...@google.com> #43
Branch: main
commit 26f726f450b61f743f7f22e17df0a1aab73980f7
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Aug 20 17:58:49 2024
Handle cloned background images correctly.
The start offset into the background image will be the same at every
fragment if box-decoration-break is 'clone'.
Bug: 40415661
Change-Id: I1619d4727e2f24d9b9d0cb6a5c1d13209834f40a
Reviewed-on:
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1344260}
M third_party/blink/renderer/core/paint/box_background_paint_context.cc
A third_party/blink/web_tests/external/wpt/css/css-break/background-image-004.html
A third_party/blink/web_tests/external/wpt/css/css-break/background-image-005.html
A third_party/blink/web_tests/external/wpt/css/css-break/background-image-006.html
A third_party/blink/web_tests/external/wpt/css/css-break/background-image-007-ref.html
A third_party/blink/web_tests/external/wpt/css/css-break/background-image-007.html
ap...@google.com <ap...@google.com> #44
Branch: main
commit ecf7c69947d4216b46f79d671e6f000a83675137
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Mon Aug 26 18:06:20 2024
Handle cloned box decorations in grid containers.
Don't assume that items resume at block-offset in a flex container
fragment after a break. There may be cloned box decorations there. Just
use BorderScrollbarPadding(). If box decorations are to be sliced, they
will be 0 there.
Also only use BorderPadding() when resolving the total node size, since
cloned box decorations are multiplied there. Use
BorderScrollbarPadding() in call to FinishFragmentation().
Items in grid containers are distributed before fragmentation, and then
adjusted afterwards. If fragmentation inserts cloned box decorations on
the grid container, we need to compensate for that when dealing with
values based on unfragmented layout. We need something almost like
"previously consumed block-size", but not quite. Previously consumed
block-size of a grid container includes any cloned box decorations, but
they are not part of the imaginary unfragmented grid container. Hence
the rename from consumed_grid_block_size to offset_in_stitched_container
in GridBreakTokenData.
Bug: 40415661
Change-Id: Id2ad6171ddeee2effadfb4bbf49907528263e8f4
Reviewed-on:
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1346839}
M third_party/blink/renderer/core/layout/grid/grid_break_token_data.h
M third_party/blink/renderer/core/layout/grid/grid_layout_algorithm.cc
M third_party/blink/renderer/core/layout/grid/grid_layout_algorithm.h
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-026.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-027.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-028.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-029.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-030.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-031.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-032-crash.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-033.html
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-034.html
ap...@google.com <ap...@google.com> #45
Branch: main
commit f7afd40fcbb68a034197b79db1923a4ce515e257
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Mon Aug 26 19:34:59 2024
Better control of breaks before trailing decorations.
Add a fragment builder flag to control this. This isn't something that
FinishFragmentation() can determine on its own. Unconditionally allowing
it for tables, like we used to, would result in an infinite loop, if the
table had already decided that we were at the last fragment, and we'd
then add a break before trailing border+padding in the cloning
box-decoration-break model.
Move some comments around, so that they are next to the code they're
relevant for.
Bug: 40415661
Change-Id: I026323731f494595a17ae6437a7454131ec9e7ce
Reviewed-on:
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1346904}
M third_party/blink/renderer/core/layout/block_layout_algorithm.cc
M third_party/blink/renderer/core/layout/box_fragment_builder.h
M third_party/blink/renderer/core/layout/fragmentation_utils.cc
M third_party/blink/renderer/core/layout/table/table_layout_algorithm.cc
A third_party/blink/web_tests/external/wpt/css/css-break/box-decoration-break-clone-035-crash.html
ms...@chromium.org <ms...@chromium.org> #46
Feature complete! Closing this.
Description
WebKit has the same issue: