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.

Sign up for GitHub

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

Jump to bottom

Made utility functions compatible with minimal script test template #3239

Merged
merged 2 commits into from
Aug 30, 2016
Merged

Made utility functions compatible with minimal script test template #3239

merged 2 commits into from
Aug 30, 2016

Conversation

tidoust
Copy link
Contributor

@tidoust tidoust commented Jun 28, 2016

The minimal script test template does not contain any tag. Browsers will implicitly create one, but only after the script runs. The mediasource_test method now checks for the existence of document.body before appending the

@tidoust
Made utility functions compatible with minimal script test template
4d97840 
The minimal script test template does not contain any <body> tag. Browsers
will implicitly create one, but only after the script runs. The mediasource_test
method now checks for the existence of document.body before appending the
<video> tag, and creates the body if needed.
@wpt-pr-bot
Copy link
Collaborator

Reviewers for this pull request are: @acolwell, @bit, @shishimaru, @sideshowbarker, and @wolenetz.

@hoppipolla-critic-bot
Copy link

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

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.

@tidoust
Copy link
Contributor Author

tidoust commented Jun 28, 2016

@wolenetz I merged #3184 without noticing that the style updates actually broke the tests, because dropping means the script is now run before the browser creates it, and thus calling document.body.appendChild will now fail.

FWIW, instead of creating the body through code, the test file could also:

  1. add the <div id="log"></div> tag back, before the last <script>, which implicitly forces the creation of <body>. That's a bit hidden, though.
  2. wrap the last <script> in a <body> tag as before
  3. add the <video> tag before the last <script>, which would have the same effect but would also make the dependency on that tag more explicit

@tidoust tidoust mentioned this pull request Jun 30, 2016
Closed
@tidoust
Fixed appendUntilEventFires to stop at the end of media segments
b738c3e 
User agents may prioritize appending media segments over the underlying
operation and expected event that the function is waiting for. The method
could crash when it reached the end of available media segments to add.
@tidoust tidoust mentioned this pull request Jul 15, 2016
Merged
@wolenetz
Copy link
Member

b738c3e makes the test believe that the event has fired even if it hasn't yet. I'll take over this PR and adjust the impl and usage of MediaSourceUtil.appendUntilEventFires to differentiate the two, while including the fix for not exceeding the segmentInfo.media array bounds.

@wolenetz
Copy link
Member

Ah. Never mind. I see that the caller already awaits the expected event, too, so we already have such differentiation. I'll merge this shortly.

@wolenetz wolenetz merged commit e9bb8c1 into web-platform-tests:master Aug 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
media-source wg-html
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tidoust @wpt-pr-bot @hoppipolla-critic-bot @wolenetz