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] Fix abspos replaced size with margins #19011

Merged
merged 1 commit into from Sep 12, 2019

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

chromium-wpt-export-bot commented Sep 11, 2019

NG was computing inline_size of replaced elements
with no intrinsic size (only aspect ratio) incorrectly.
Element's margin was not considered.

The fixes are:

  • update code to use margin when computing inline_size
  • update wpt exaustive replaced size test to include margins
  • remove many stale expectations for position-absolute-replaced-minmax.html

With the updated test, Legacy still fails single test. I might
have to update expectations.

Bug: 1002748
Change-Id: I98b8595ce0e4dc0d19d1d8020e33fe17e125d41d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799422
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696101}

Copy link
Collaborator

wpt-pr-bot left a comment

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1799422 branch 4 times, most recently from 5a8e55d to 0929b87 Sep 11, 2019
NG was computing inline_size of replaced elements
with no intrinsic size (only aspect ratio) incorrectly.
Element's margin was not considered.

The fixes are:
- update code to use margin when computing inline_size
- update wpt exaustive replaced size test to include margins
- remove many stale expectations for position-absolute-replaced-minmax.html

With the updated test, Legacy still fails single test. I might
have to update expectations.

Bug: 1002748
Change-Id: I98b8595ce0e4dc0d19d1d8020e33fe17e125d41d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799422
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696101}
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1799422 branch from 0929b87 to 2d589a3 Sep 12, 2019
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit dbf3183 into master Sep 12, 2019
14 checks passed
14 checks passed
update-pr-preview
Details
staging.wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - chrome[experimental] Chrome results
Details
Azure Pipelines Build #20190912.138 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Taskcluster (pull_request) TaskGroup: success
Details
staging.wpt.fyi - firefox[experimental] Firefox results
Details
staging.wpt.fyi - safari[experimental] Safari results
Details
wpt.fyi - firefox[experimental] Firefox results
Details
wpt.fyi - safari[experimental] Safari results
Details
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-1799422 branch Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.