[PE] Fix LayoutBox Client/Padding/Content boxes with scrollbars (especially vertical-rl) #12745
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
approved these changes
Aug 29, 2018
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-1185901
branch
3 times, most recently
from
1a23916
to
f2a9f3e
Compare
…cially 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}
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-1185901
branch
from
f2a9f3e
to
2f3f85d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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}