Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LayoutNG] Don't resolve unresolvable percentage min-height and max-height. #10019

Merged
merged 1 commit into from Mar 14, 2018

Conversation

Projects
None yet
4 participants
@chromium-wpt-export-bot
Copy link
Collaborator

chromium-wpt-export-bot commented Mar 13, 2018

We cannot use the content size (intrinsic block size) as min-height or
max-height (min-width or max-width, if writing mode is vertical).
Doing so would cause min-height to incorrectly override any specified
height or max-height that is less than the intrinsic block size, and
max-height to override any specified height that's greater than the
intrinsic block size. Do what the spec says [1] instead: max-height
to become 'none', and min-height to become 0.

This fixes the final rendering problem with Acid2 (this was about the
min-height issue). It now renders correctly with LayoutNG!

Since no other tests than Acid2 started to pass with this change, I wrote
a couple.

[1] https://www.w3.org/TR/CSS22/visudet.html#min-max-heights

 XXXXXX

XX XX
X X
X O O X
X X
X /\ X
X / X
X X
X X X X
X XXXXXXXX X
X X
XX XX
XXXXXX

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ic4f2c3c1dbc2bd66956ed4b270b08058999bc351
Reviewed-on: https://chromium-review.googlesource.com/960085
Reviewed-by: Ian Kilpatrick ikilpatrick@chromium.org
Reviewed-by: Aleks Totic atotic@chromium.org
Reviewed-by: Emil A Eklund eae@chromium.org
Commit-Queue: Morten Stenshorne mstensho@chromium.org
Cr-Commit-Position: refs/heads/master@{#543018}

@wpt-pr-bot
Copy link
Collaborator

wpt-pr-bot left a comment

Already reviewed downstream.

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 13, 2018

Build PASSED

Started: 2018-03-14 06:41:25
Finished: 2018-03-14 06:46:10

View more information about this build on:

[LayoutNG] Don't resolve unresolvable percentage min-height and max-h…
…eight.

We cannot use the content size (intrinsic block size) as min-height or
max-height (min-width or max-width, if writing mode is vertical).
Doing so would cause min-height to incorrectly override any specified
height or max-height that is less than the intrinsic block size, and
max-height to override any specified height that's greater than the
intrinsic block size. Do what the spec says [1] instead: max-height
to become 'none', and min-height to become 0.

This fixes the final rendering problem with Acid2 (this was about the
min-height issue). It now renders correctly with LayoutNG!

Since no other tests than Acid2 started to pass with this change, I wrote
a couple.

[1] https://www.w3.org/TR/CSS22/visudet.html#min-max-heights

     XXXXXX
   XX      XX
  X          X
 X   O    O   X
 X            X
X      /\      X
X      \/      X
X              X
 X X        X X
 X  XXXXXXXX  X
  X          X
   XX      XX
     XXXXXX

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ic4f2c3c1dbc2bd66956ed4b270b08058999bc351
Reviewed-on: https://chromium-review.googlesource.com/960085
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543018}

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-960085 branch from eb3a3c4 to 2eafc77 Mar 14, 2018

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 6573773 into master Mar 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-960085 branch Mar 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.