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

Fix #8443 #8496

Merged
merged 2 commits into from Mar 30, 2016
Merged

Fix #8443 #8496

merged 2 commits into from Mar 30, 2016

Conversation

@notriddle
Copy link
Contributor

notriddle commented Nov 12, 2015

When there are viewport constraints, use the new window size to compute the viewport instead of the old one.

Review on Reviewable

@eefriedman eefriedman self-assigned this Nov 13, 2015
@eefriedman
Copy link
Contributor

eefriedman commented Nov 13, 2015

Do you have to do something to make the test harness ignore meta_viewport_resize_iframe.html? Or am I getting things mixed up?

@notriddle
Copy link
Contributor Author

notriddle commented Nov 13, 2015

It's not in the manifest. Is it being automatically added? If so, we can
convert it into a data URL.

On Thu, Nov 12, 2015, 18:37 eefriedman notifications@github.com wrote:

Do you have to do something to make the test harness ignore
meta_viewport_resize_iframe.html? Or am I getting things mixed up?


Reply to this email directly or view it on GitHub
#8496 (comment).

@eefriedman
Copy link
Contributor

eefriedman commented Nov 13, 2015

You can see if the harness thinks the current manifest is correct by running something like ./mach test-wpt --update-manifest.

@jdm
Copy link
Member

jdm commented Nov 13, 2015

My understanding is that only files containing <link rel=match href=...> and <script src=/resources/testharness.js> get added to the manifest automatically.

@eefriedman
Copy link
Contributor

eefriedman commented Nov 13, 2015

Oh, I didn't realize it was quite that smart. Okay, thanks.

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Nov 13, 2015

📌 Commit c770908 has been approved by eefriedman

@bors-servo
Copy link
Contributor

bors-servo commented Nov 13, 2015

Testing commit c770908 with merge 3d50c03...

bors-servo added a commit that referenced this pull request Nov 13, 2015
Fix #8443

When there are viewport constraints, use the new window size to compute the viewport instead of the old one.

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

bors-servo commented Nov 13, 2015

💔 Test failed - mac-rel-wpt

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 13, 2015

Review status: 0 of 4 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


tests/wpt/mozilla/tests/css/meta_viewport_resize.html, line 10 [r2] (raw file):
This would be better as an async_test.


tests/wpt/mozilla/tests/css/meta_viewport_resize.html, line 18 [r2] (raw file):
Remove the console.log call.


tests/wpt/mozilla/tests/css/meta_viewport_resize.html, line 22 [r2] (raw file):
What's this timeout based on?


Comments from the review on Reviewable.io

@notriddle
Copy link
Contributor Author

notriddle commented Nov 13, 2015

tests/wpt/mozilla/tests/css/meta_viewport_resize.html, line 22 [r2] (raw file):
The number is pretty much arbitrary; the point is to wait until reflow is done.

requestAnimationFrame works when I tried it on by box, so let's see if the Mac agrees.


Comments from the review on Reviewable.io

@eefriedman eefriedman assigned Ms2ger and unassigned eefriedman Nov 13, 2015
@notriddle
Copy link
Contributor Author

notriddle commented Nov 14, 2015

@jdm
Copy link
Member

jdm commented Nov 14, 2015

@notriddle Please make a PR to servo/saltfs to add yourself to the list of users with try access.

@jdm
Copy link
Member

jdm commented Nov 14, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Nov 14, 2015

Trying commit 94463a7 with merge 086f225...

bors-servo added a commit that referenced this pull request Nov 14, 2015
Fix #8443

When there are viewport constraints, use the new window size to compute the viewport instead of the old one.

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

bors-servo commented Nov 14, 2015

💔 Test failed - mac-rel-wpt

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

📌 Commit 18dfef0 has been approved by notriddle

@jdm
Copy link
Member

jdm commented Mar 29, 2016

@jdm
Copy link
Member

jdm commented Mar 29, 2016

There's a trailing space in MANIFEST.json.

@notriddle
Copy link
Contributor Author

notriddle commented Mar 29, 2016

Fixed that 🤦

@bors-servo r=jdm

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

📌 Commit ef571ef has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

Testing commit ef571ef with merge d0b2089...

bors-servo added a commit that referenced this pull request Mar 29, 2016
Fix #8443

When there are viewport constraints, use the new window size to compute the viewport instead of the old one.

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

bors-servo commented Mar 29, 2016

💔 Test failed - status-appveyor

@notriddle
Copy link
Contributor Author

notriddle commented Mar 29, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

💣 Failed to start rebuilding: Unknown error

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

Testing commit ef571ef with merge 210a243...

bors-servo added a commit that referenced this pull request Mar 29, 2016
Fix #8443

When there are viewport constraints, use the new window size to compute the viewport instead of the old one.

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

bors-servo commented Mar 30, 2016

@bors-servo bors-servo merged commit ef571ef into servo:master Mar 30, 2016
3 checks passed
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
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

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