Skip to content

/ servo

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

Fully implement the "align descendants" rule for div. #7825

Merged
merged 2 commits into from Oct 8, 2015
Merged

Fully implement the "align descendants" rule for div. #7825

merged 2 commits into from Oct 8, 2015
+194 −40

Conversation

@eefriedman
Copy link
Contributor

eefriedman commented Oct 2, 2015

This adds -servo-left and -servo-right to complement -servo-center.

This intentionally doesn't try to address issue #7301. Commit added to address #7301.

Review on Reviewable
@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 2, 2015

This test should go somewhere under tests/wpt/web-platform-tests/html/rendering/.
@eefriedman
Copy link
Contributor Author

eefriedman commented Oct 2, 2015

tests/wpt/web-platform-tests/html/rendering/ looks like it's part of an upstream repository; do you want me to upstream this test?

I'm guess the test should be named something like web-platform-tests/html/rendering/non-replaced-elements/flow-content-0/div-align.html?

Does it make sense to upstream the test as-is, without a test for #7301?
@jdm
Copy link
Member

jdm commented Oct 2, 2015

Upstreaming happens automatically, so it's fine to make changes there.
eefriedman added 2 commits Oct 2, 2015
@eefriedman
Fully implement the "align descendants" rule for div.
69ca066 
This adds -servo-left and -servo-right to complement -servo-center.

This intentionally doesn't try to address issue #7301.
@eefriedman
Make "align descendants" rule use parent's text-align.
Loading status checks…
a31461b 
Flows never care about their own text-align, only the text-align of
their parent; change the text-align flags to account for that.  Make the
"align descendants" rule use the flags instead of the current node's style.

Fixes #7301.
@eefriedman eefriedman force-pushed the eefriedman:div-align branch from bd9cabe to a31461b Oct 2, 2015
@eefriedman
Copy link
Contributor Author

eefriedman commented Oct 2, 2015

Moved tests. Added fix for #7301, since it turned out to be easier than I thought it would be.
@mbrubeck mbrubeck self-assigned this Oct 2, 2015
@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 8, 2015

Reviewed 8 of 8 files at r1, 2 of 2 files at r2.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from the review on Reviewable.io
@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 8, 2015

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

📌 Commit a31461b has been approved by mbrubeck
@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

⌛ Testing commit a31461b with merge ab42ca4...
bors-servo pushed a commit that referenced this pull request Oct 8, 2015
bors-servo
Auto merge of #7825 - eefriedman:div-align, r=mbrubeck
Loading status checks…
ab42ca4 
Fully implement the "align descendants" rule for div.

This adds -servo-left and -servo-right to complement -servo-center.

~~This intentionally doesn't try to address issue #7301.~~  Commit added to address #7301.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7825)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

💔 Test failed - mac-rel-wpt
@jdm
Copy link
Member

jdm commented Oct 8, 2015

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

⚡ Previous build results for android, gonk, linux-dev, linux-rel, mac-dev-ref-unit are reusable. Rebuilding only mac-rel-css, mac-rel-wpt...
@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit a31461b into servo:master Oct 8, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@eefriedman eefriedman mentioned this pull request Oct 10, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mbrubeck mbrubeck

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@eefriedman @Ms2ger @jdm @mbrubeck @bors-servo @SimonSapin @highfive
You can’t perform that action at this time.