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

Don't use image metadata for layout when rendering to a file #9754

Merged
merged 1 commit into from Feb 25, 2016
Merged

Don't use image metadata for layout when rendering to a file #9754

merged 1 commit into from Feb 25, 2016

Conversation

@mbrubeck
Copy link
Contributor

mbrubeck commented Feb 25, 2016

#9208 added the ability to perform layout before an <img> resource is fully loaded. However, when we are rendering to an image file for testing purposes, we need to block until the image content is fully loaded (until #9441 is fixed).

Fixes #9550. r? @jdm

Review on Reviewable
@highfive
Copy link

highfive commented Feb 25, 2016

warning Warning warning

  • These commits modify layout code, but no tests are modified. Please consider adding a test!
@mbrubeck mbrubeck added the A-testing label Feb 25, 2016
@jdm
Copy link
Member

jdm commented Feb 25, 2016

Nice simplification!
-S-awaiting-review +S-needs-code-changes

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

components/layout/context.rs, line 186 [r1] (raw file):
Let's move this check above the let result to avoid wasted work.

Comments from the review on Reviewable.io
@mbrubeck
Don't use image metadata for layout when rendering to a file
Loading status checks…
37dd2a6 
PR #9208 added the ability to perform layout before an `<img>` resource is
fully loaded. However, when we are rendering to an image file for testing
purposes, we need to block until the image content is fully loaded (until
issue #9441 is fixed).

Fixes #9550.
@mbrubeck mbrubeck force-pushed the mbrubeck:img-metadata branch from fa22bf2 to 37dd2a6 Feb 25, 2016
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Feb 25, 2016

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.

components/layout/context.rs, line 186 [r1] (raw file):
Done.

Comments from the review on Reviewable.io
@jdm
Copy link
Member

jdm commented Feb 25, 2016

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

bors-servo commented Feb 25, 2016

📌 Commit 37dd2a6 has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Feb 25, 2016

⌛ Testing commit 37dd2a6 with merge 21f0e63...
bors-servo added a commit that referenced this pull request Feb 25, 2016
@bors-servo
Auto merge of #9754 - mbrubeck:img-metadata, r=jdm
Loading status checks…
21f0e63 
Don't use image metadata for layout when rendering to a file

Fixes #9550. r? @jdm

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

bors-servo commented Feb 25, 2016

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit 37dd2a6 into servo:master Feb 25, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Ms2ger Ms2ger mentioned this pull request Apr 14, 2016
Open
@mbrubeck mbrubeck deleted the mbrubeck:img-metadata branch May 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jdm jdm

Labels
A-content/images A-testing S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Intermittent FAIL [expected PASS] /_mozilla/css/filter_opacity_a.html
4 participants
@mbrubeck @highfive @jdm @bors-servo
You can’t perform that action at this time.