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

Fix table vertical alignment (middle, bottom) #12593

Merged
merged 1 commit into from Jul 27, 2016

Conversation

Projects
None yet
7 participants
@adamncasey
Contributor

adamncasey commented Jul 25, 2016

Fixes table cell vertical alignment (middle, bottom, not yet baseline) when the row contains cells of differing heights.

Moved the work done earlier by @notriddle into a separate public function on TableCellFlow. This function is then called by TableRowFlow once the cell's block size has been calculator.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #12531 (github issue number if applicable).
  • There are tests for these changes

This change is Reviewable

@adamncasey

This comment has been minimized.

Contributor

adamncasey commented Jul 25, 2016

Have run ./mach test-wpt tests/wpt/mozilla/tests/mozilla/ locally. Travis might throw up some other edge cases I haven't thought about

@pcwalton

This comment has been minimized.

Contributor

pcwalton commented Jul 25, 2016

@bors-servo: r+

This looks like the right fix to me. Thanks!

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 25, 2016

📌 Commit 585656d has been approved by pcwalton

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 26, 2016

⌛️ Testing commit 585656d with merge b75a5bf...

bors-servo added a commit that referenced this pull request Jul 26, 2016

Auto merge of #12593 - adamncasey:table-row-cell-height, r=pcwalton
Fix table vertical alignment (middle, bottom)

<!-- Please describe your changes on the following line: -->
Fixes table cell vertical alignment (middle, bottom, not yet baseline) when the row contains cells of differing heights.

Moved the work done earlier by @notriddle into a separate public function on TableCellFlow. This function is then called by TableRowFlow once the cell's block size has been calculator.

---

<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12531 (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12593)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 26, 2016

💔 Test failed - linux-rel

@highfive

This comment has been minimized.

highfive commented Jul 26, 2016

  ▶ PASS [expected FAIL] /css21_dev/html4/margin-collapse-clear-002.htm

  ▶ PASS [expected FAIL] /css21_dev/html4/margin-collapse-clear-003.htm

  ▶ PASS [expected FAIL] /css21_dev/html4/margin-collapse-clear-009.htm

  ▶ PASS [expected FAIL] /css21_dev/html4/margin-collapse-clear-008.htm
@adamncasey

This comment has been minimized.

Contributor

adamncasey commented Jul 26, 2016

reftest rendering

The references for these tests use tables, and the left column's vertical alignment does not take into account the border height of the right column (which makes the columns uneven heights). This patch fixes this.

Fix table vertical alignment (middle, bottom, not yet baseline) with …
…differing height cells

Includes reftest for this behaviour
Patch fixes margin-collapse-clear-002,3,8,9
@jdm

This comment has been minimized.

Member

jdm commented Jul 26, 2016

@bors-servo: r=pcwalton

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 26, 2016

📌 Commit b1debc4 has been approved by pcwalton

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 26, 2016

⌛️ Testing commit b1debc4 with merge 4f4ba4a...

bors-servo added a commit that referenced this pull request Jul 26, 2016

Auto merge of #12593 - adamncasey:table-row-cell-height, r=pcwalton
Fix table vertical alignment (middle, bottom)

<!-- Please describe your changes on the following line: -->
Fixes table cell vertical alignment (middle, bottom, not yet baseline) when the row contains cells of differing heights.

Moved the work done earlier by @notriddle into a separate public function on TableCellFlow. This function is then called by TableRowFlow once the cell's block size has been calculator.

---

<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12531 (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12593)
<!-- Reviewable:end -->

@bors-servo bors-servo referenced this pull request Jul 26, 2016

Merged

Make the service worker send custom response #12582

3 of 4 tasks complete
@larsbergstrom

This comment has been minimized.

Contributor

larsbergstrom commented Jul 26, 2016

@bors-servo retry clean force

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 26, 2016

⌛️ Testing commit b1debc4 with merge 148c8e3...

bors-servo added a commit that referenced this pull request Jul 26, 2016

Auto merge of #12593 - adamncasey:table-row-cell-height, r=pcwalton
Fix table vertical alignment (middle, bottom)

<!-- Please describe your changes on the following line: -->
Fixes table cell vertical alignment (middle, bottom, not yet baseline) when the row contains cells of differing heights.

Moved the work done earlier by @notriddle into a separate public function on TableCellFlow. This function is then called by TableRowFlow once the cell's block size has been calculator.

---

<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12531 (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12593)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 26, 2016

💔 Test failed - mac-rel-css

@adamncasey

This comment has been minimized.

Contributor

adamncasey commented Jul 26, 2016

I think the stdio output for the failed build is:

looking for rustc at /Users/servo/.servo/rust/2016-07-25/rustc-nightly-x86_64-apple-darwin/rustc/bin/rustc
Downloading Rust compiler...
Extracting Rust compiler...
Rust compiler ready.
Downloading Host rust library for target x86_64-apple-darwin...
Extracting Rust stdlib for target x86_64-apple-darwin...
Rust x86_64-apple-darwin libs ready.
Build completed in 0:00:09

I'm not sure what's going on

@larsbergstrom

This comment has been minimized.

Contributor

larsbergstrom commented Jul 26, 2016

@bors-servo retry

  • mac1 is still being provisioned

bors-servo added a commit that referenced this pull request Jul 27, 2016

Auto merge of #12593 - adamncasey:table-row-cell-height, r=pcwalton
Fix table vertical alignment (middle, bottom)

<!-- Please describe your changes on the following line: -->
Fixes table cell vertical alignment (middle, bottom, not yet baseline) when the row contains cells of differing heights.

Moved the work done earlier by @notriddle into a separate public function on TableCellFlow. This function is then called by TableRowFlow once the cell's block size has been calculator.

---

<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12531 (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12593)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 27, 2016

⌛️ Testing commit b1debc4 with merge b374582...

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Jul 27, 2016

@bors-servo bors-servo merged commit b1debc4 into servo:master Jul 27, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@adamncasey adamncasey deleted the adamncasey:table-row-cell-height branch Jul 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment