Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 1023179 - Fix video unloading/loading, refactor initialization #20501

Merged
merged 1 commit into from Jun 25, 2014

Conversation

sfoster
Copy link
Contributor

@sfoster sfoster commented Jun 13, 2014

Refactor of initialization sequence, logging to track down media loading issue

@try-server-hook
Copy link

Continuous Integration started. Results

@try-server-hook
Copy link

Continuous Integration started. Results

@sfoster sfoster changed the title Bug 1023179 - tutorial tests exploration Bug 1023179 - Fix video unloading/loading, refactor initialization Jun 16, 2014
if (typeof callback === 'function') {
videoElement.addEventListener('loadeddata', onVideoLoadOrError, false);
videoElement.addEventListener('error', onVideoLoadOrError, false);
function loadMedia(mediaElement, src) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is a private function, please name it as _loadMedia (it's just a convention)

@try-server-hook
Copy link

Continuous Integration started. Results

@sfoster
Copy link
Contributor Author

sfoster commented Jun 20, 2014

I did a ton more testing today. It seems calling mediaElement.load() after removing the src attribute is necessary to full unload the current video, and only after the emptied event is it reliably safe to assign a new src. I think I've addressed the other comments either above or in the new PR.

} else {
evt.target.onload = evt.target.onerror = null;
}
if (evt.type == 'error') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use better ===

@try-server-hook
Copy link

Continuous Integration started. Results

onUnloaded();
}
} else {
mediaElement.onload = onMediaLoadOrError;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mediaElement.onload = mediaElement.onerror = onMediaLoadOrError;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a question about this, why are we using event listeners for the video, but inline events for images? any specific reason?

@try-server-hook
Copy link

Continuous Integration started. Results

@sfoster
Copy link
Contributor Author

sfoster commented Jun 24, 2014

I updated the PR to use event listeners for the image case as well - it was inconsistent. If the try run comes up green I'll carry the r+ and land.

@try-server-hook
Copy link

Continuous Integration started. Results

… r=fcampo

* Fix sporadic unencoding error by unloading video before loading new src.
* Refactor Tutorial init to eliminate race-conditions and better manage mixed sequence of asnyc and sync steps
* Use mock XHR for all tutorial tests
* Catch emptied/abort event when unloading video before assigning new src
* Use canplay event to signal video media loadedSignal media loaded
@try-server-hook
Copy link

Continuous Integration started. Results

sfoster added a commit that referenced this pull request Jun 25, 2014
Bug 1023179  - Fix FTU video unloading/loading, refactor tutorial initialization. r=fcampo
@sfoster sfoster merged commit 82cccd1 into mozilla-b2g:master Jun 25, 2014
KWierso added a commit that referenced this pull request Jun 25, 2014
…ests" for Gaia-ui test failures

This reverts commit 82cccd1, reversing
changes made to bd5888a.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants