Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign uplayout: Implement `border-spacing` per CSS 2.1 § 17.6.1 and the legacy `cellspacing` attribute per HTML5 § 14.3.9. #4417
Conversation
|
Looks like this conflicts with master already. Please rebase before Critic picks it up. |
|
Rebased. |
pcwalton
added
S-awaiting-review
A-layout/uncategorized
I-enhancement
and removed
S-needs-rebase
labels
hoppipolla-critic-bot
commented
Dec 18, 2014
|
Critic review: https://critic.hoppipolla.co.uk/r/3542 This is an external review system which you may optionally use for the code review of your pull request. In order to help critic track your changes, please do not make in-place history rewrites (e.g. via |
|
Rebased. r? @SimonSapin |
pcwalton
force-pushed the
pcwalton:border-spacing
branch
2 times, most recently
from
5433b01
to
82944bc
|
Rebased. r? @SimonSapin |
|
Critic has 14 open issues on this PR, it looks like they haven’t been addressed. Rebasing in the middle of a review makes the rest of the review harder :( |
|
Addressed issues. The tests you asked me to add actually exposed some bugs in automatic and fixed table layout independent of spacing, so I fixed those too. r? @SimonSapin |
|
The issues appear to have been addressed and the additional nasty issue fixes appear correct, to my understanding of the table layout code. Please squash and r+! |
`cellspacing` attribute per HTML5 § 14.3.9. Table layout code has been refactored to push the spacing down to rowgroups and rows; this will aid the implementation of `border-collapse` as well. This commit also fixes two nasty issues in table layout: * In fixed layout, extra space would not be divided among columns that had auto width but had nonzero minimum width. * In automatic layout, extra space would be distributed to constrained columns as well even if unconstrained columns with percentage equal to zero were present.
This comment has been minimized.
This comment has been minimized.
|
r=larsbergstrom |
This comment has been minimized.
This comment has been minimized.
|
saw approval from larsbergstrom |
This comment has been minimized.
This comment has been minimized.
|
merging pcwalton/servo/border-spacing = 586c12c into auto |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
all tests pass: |
This comment has been minimized.
This comment has been minimized.
|
fast-forwarding master to auto = 8e81122 |
Table layout code has been refactored to push the spacing down to rowgroups and rows; this will aid the implementation of `border-collapse` as well. r? @SimonSapin
bors-servo
merged commit 586c12c
into
servo:master
pcwalton commentedDec 18, 2014
Table layout code has been refactored to push the spacing down to
rowgroups and rows; this will aid the implementation of
border-collapseas well.r? @SimonSapin