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

Fix htmlmediaelement::response_complete #12671

Closed
wants to merge 1 commit into from
Closed

Fix htmlmediaelement::response_complete #12671

wants to merge 1 commit into from

Conversation

@GuillaumeGomez
Copy link
Contributor

GuillaumeGomez commented Jul 31, 2016 •
edited by larsbergstrom

This change creates a segfault on my computer. No idea why... Let's see what travis says.

This change is Reviewable
@GuillaumeGomez
Fix htmlmediaelement::response_complete
Loading status checks…
1771b71
@highfive
Copy link

highfive commented Jul 31, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/htmlmediaelement.rs
@highfive
Copy link

highfive commented Jul 31, 2016

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@jdm
Copy link
Member

jdm commented Jul 31, 2016

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

bors-servo commented Jul 31, 2016

⌛ Trying commit 1771b71 with merge cbacb35...
bors-servo added a commit that referenced this pull request Jul 31, 2016
@bors-servo
Auto merge of #12671 - GuillaumeGomez:fix_reponse_complete, r=<try>
Loading status checks…
cbacb35 
Fix htmlmediaelement::response_complete

This change creates a segfault on my computer. No idea why... Let's see what travis says.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12671)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 31, 2016

💔 Test failed - linux-rel
@GuillaumeGomez
Copy link
Contributor Author

GuillaumeGomez commented Jul 31, 2016

Totally unexpected failures. :-/
@jdm
Copy link
Member

jdm commented Jul 31, 2016

Looks like a lot of audio-related failures again.
@GuillaumeGomez
Copy link
Contributor Author

GuillaumeGomez commented Jul 31, 2016

Which is problematic since I don't understand why it fails. It isn't supposed to work currently, right?
@cbrewster
Copy link
Member

cbrewster commented Aug 17, 2016

@GuillaumeGomez any plans to investigate the test failures here?
@GuillaumeGomez
Copy link
Contributor Author

GuillaumeGomez commented Aug 17, 2016

I am but any help is welcome.
@jdm
Copy link
Member

jdm commented Sep 6, 2016

I believe the failures are expected - we had no code that could retrieve metadata for audio files, so any media element loading now doesn't dispatch a bunch of events that the tests rely on in order to complete.
@KiChjang
Copy link
Member

KiChjang commented Sep 22, 2016

@GuillaumeGomez Any plans on finishing this?
@GuillaumeGomez
Copy link
Contributor Author

GuillaumeGomez commented Sep 22, 2016

@KiChjang: Yes but I don't know when.
@jdm jdm mentioned this pull request Sep 22, 2016
Closed
@jdm
Copy link
Member

jdm commented Sep 22, 2016

Closing in favour of #13375, in that case.
@jdm jdm closed this Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cbrewster cbrewster

Labels
S-needs-code-changes
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@GuillaumeGomez @highfive @jdm @bors-servo @cbrewster @KiChjang
You can’t perform that action at this time.