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

layout: Modify styles for replaced content as appropriate during incremental flow construction. #6492

Merged
merged 4 commits into from Jul 7, 2015

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Jun 27, 2015

Fixes jumpiness on lots of Web sites.

r? @mbrubeck

Review on Reviewable

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jun 27, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5395

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 git rebase -i or git commit --amend) when updating this pull request.

@mbrubeck
Copy link
Contributor

mbrubeck commented Jun 27, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Jun 27, 2015

📌 Commit ef34491 has been approved by mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jun 27, 2015

Testing commit ef34491 with merge f4f864d...

bors-servo pushed a commit that referenced this pull request Jun 27, 2015
layout: Modify styles for replaced content as appropriate during incremental flow construction.

Fixes jumpiness on lots of Web sites.

r? @mbrubeck

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

bors-servo commented Jun 27, 2015

💔 Test failed - linux3

@pcwalton
Copy link
Contributor Author

pcwalton commented Jun 29, 2015

@bors-servo: r=mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jun 29, 2015

📌 Commit 6775e36 has been approved by mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jun 29, 2015

Testing commit 6775e36 with merge b49cebd...

bors-servo pushed a commit that referenced this pull request Jun 29, 2015
layout: Modify styles for replaced content as appropriate during incremental flow construction.

Fixes jumpiness on lots of Web sites.

r? @mbrubeck

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

bors-servo commented Jun 29, 2015

💔 Test failed - linux3

@pcwalton pcwalton force-pushed the pcwalton:even-more-jumpiness branch from 6775e36 to 22c8281 Jun 29, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jun 29, 2015

@bors-servo: r=mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jun 29, 2015

📌 Commit 22c8281 has been approved by mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jun 29, 2015

Testing commit 22c8281 with merge a14ca84...

bors-servo pushed a commit that referenced this pull request Jun 29, 2015
layout: Modify styles for replaced content as appropriate during incremental flow construction.

Fixes jumpiness on lots of Web sites.

r? @mbrubeck

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

bors-servo commented Jun 29, 2015

💔 Test failed - mac3

@jdm
Copy link
Member

jdm commented Jul 6, 2015

This looks like insert-inline-in-blocks-n-inlines-end-001.htm was incorrectly marked as passing now...

@jdm jdm added S-tests-failed and removed S-awaiting-review labels Jul 6, 2015
@pcwalton pcwalton force-pushed the pcwalton:even-more-jumpiness branch from 22c8281 to 5ad1442 Jul 6, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 6, 2015

@bors-servo: r=mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2015

📌 Commit 5ad1442 has been approved by mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2015

Testing commit 5ad1442 with merge 20053a1...

@pcwalton pcwalton force-pushed the pcwalton:even-more-jumpiness branch from 8092d31 to 3145497 Jul 7, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 7, 2015

@bors-servo: r=mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2015

📌 Commit 3145497 has been approved by mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2015

Testing commit 3145497 with merge d7f2c52...

bors-servo pushed a commit that referenced this pull request Jul 7, 2015
layout: Modify styles for replaced content as appropriate during incremental flow construction.

Fixes jumpiness on lots of Web sites.

r? @mbrubeck

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

bors-servo commented Jul 7, 2015

💔 Test failed - mac3

@jdm
Copy link
Member

jdm commented Jul 7, 2015

Eep.

/css21_dev/html4/block-in-inline-001.htm
----------------------------------------
FAIL [Parent]
/css21_dev/html4/block-in-inline-002.htm
----------------------------------------
FAIL [Parent]
@jdm jdm added S-tests-failed and removed S-awaiting-merge labels Jul 7, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 7, 2015

Platform specific failure again, I guess?

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2015

The latest upstream changes (presumably #6487) made this pull request unmergeable. Please resolve the merge conflicts.

pcwalton added 4 commits Jun 27, 2015
incremental flow construction.

Fixes jumpiness on lots of Web sites.
Fixes `insert-inline-in-blocks-n-inlines-end-001` in the WPT tests.
@pcwalton pcwalton force-pushed the pcwalton:even-more-jumpiness branch from 3145497 to 54b9aa2 Jul 7, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 7, 2015

I started ignoring the failing tests. They seem to depend on the height above the baseline being equal to the ascent of the font in order to avoid a pixel or two of red showing through. This is not always the case in Gecko or Servo, so they fail in these two browsers. I believe the problem is with the test.

I think they were passing before because the incorrect rounding of rects just happened to make them pass, though the logic that they were testing was never correct—another reason to ignore them.

r? @mbrubeck

@mbrubeck
Copy link
Contributor

mbrubeck commented Jul 7, 2015

Reviewed 4 of 4 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, 23 of 23 files at r4.
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 Jul 7, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2015

📌 Commit 54b9aa2 has been approved by mbrubeck

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2015

Testing commit 54b9aa2 with merge bbcd427...

bors-servo pushed a commit that referenced this pull request Jul 7, 2015
layout: Modify styles for replaced content as appropriate during incremental flow construction.

Fixes jumpiness on lots of Web sites.

r? @mbrubeck

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

bors-servo commented Jul 7, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

@bors-servo bors-servo merged commit 54b9aa2 into servo:master Jul 7, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.