Closed Bug 880571 Opened 9 years ago Closed 9 years ago

[Video] Don't display error message for invalid files when video app is playing videos from internet

Categories

(Firefox OS Graveyard :: Gaia::Video, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:leo+, b2g18 fixed)

RESOLVED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed

People

(Reporter: dkuo, Assigned: dkuo)

References

Details

Attachments

(1 file)

In bug 874289 we handled the errors for playing invalid files, but those error messages should be only for local files on storages, not for the videos from internet like youtube.

We can fix this by registering the onerror function only when playing a file, not a URL.
Duplicate of this bug: 882780
Summary: [Video] Don't display error message for invalid files when video app is playing youtube videos from internet → [Video] Don't display error message for invalid files when video app is playing videos from internet
Blocks: 881388
According to the below links:
http://dev.w3.org/html5/spec-author-view/video.html#error-codes
http://dxr.allizom.org/mozilla-central/source/dom/interfaces/html/nsIDOMMediaError.idl

There are four kinds of error that a video element has, which are:
MEDIA_ERR_ABORTED
MEDIA_ERR_NETWORK
MEDIA_ERR_DECODE
MEDIA_ERR_SRC_NOT_SUPPORTED

And for playing local/remote videos, the error handlings should be both applicable, so the patch I implemented is to see what the error code is and display the corresponding error messages. I haven't decide the error messages yet and I would like some feedback from David or Sotaro cause they should be more familiar with video element than I am.
Attachment #763497 - Flags: feedback?(sotaro.ikeda.g)
Attachment #763497 - Flags: feedback?(dflanagan)
I feel following categorization is reasonable. 
- MEDIA_ERR_DECODE and MEDIA_ERR_SRC_NOT_SUPPORTED use same error message as the current message.  
- Show network error message in MEDIA_ERR_NETWORK case.
- Do not show error message in MEDIA_ERR_ABORTED case. It is issued by cancellation by document or element.
Dominic,

I've added some comments on github. 

If I recall correctly, I'm the one who alerted you to this case. But I don't remember what I did to cause an error. Maybe I tried to view a youtube video over a very bad data connection and got a network error?  

Have you been able to reproduce the issue?  If so, you can probably create an error message based on how you reproduce it.
Comment on attachment 763497 [details]
handle error codes for video element

Oops. I offered feedback in a regular comment and forgot to set this flag.

I think this look good, except that you may be trying to distinguish too many different kinds of errors.  If we had a more specific steps-to-reproduce, it would be easier to say what the right error message should be.
Attachment #763497 - Flags: feedback?(dflanagan) → feedback+
Comment on attachment 763497 [details]
handle error codes for video element

feedback+. It is better to check which error is generated in actual device.
Attachment #763497 - Flags: feedback?(sotaro.ikeda.g) → feedback+
Blocking on 880571 in support of bug 881388.
blocking-b2g: --- → leo+
Thanks Sotato and David, I will combine your feedbacks to revise my patch.

(In reply to David Flanagan [:djf] from comment #4)
> If I recall correctly, I'm the one who alerted you to this case. But I don't
> remember what I did to cause an error. Maybe I tried to view a youtube video
> over a very bad data connection and got a network error?  

Yes, a bad data connection will cause this issue, and a simple way to reproduce an error with MEDIA_ERR_NETWORK is to disconnect/disable the Wi-Fi while watching a youtube video. That's how I tested my patch.
Comment on attachment 763497 [details]
handle error codes for video element

David,

I have revised my patch base on Sotaro's feedback and yours, and it's ready for review. I have tested three scenarios to trigger the MEDIA_ERR_NETWORK, MEDIA_ERR_DECODE and MEDIA_ERR_SRC_NOT_SUPPORTED errors, which are:

1. MEDIA_ERR_NETWORK: disable the Wi-Fi while watching a youtube video.
2. MEDIA_ERR_DECODE: use bluetooth to transfer an unsupported video(I used HD videos) then try to play it.
3. MEDIA_ERR_SRC_NOT_SUPPORTED: tap a youtube link in browser app when device is offline.

And for MEDIA_ERR_ABORTED, it will just ignore sliently, but I don't know how to test this one so probably you can try to reproduce it if you are interested in it.
Attachment #763497 - Flags: review?(dflanagan)
Blocks bug 881388, so this is a QE3 bug.
Target Milestone: --- → 1.1 QE3 (24jun)
Priority: -- → P1
Comment on attachment 763497 [details]
handle error codes for video element

Looks like David is busy and Dale is in Taipei mini workweek now, just asked him and he can help on reviewing this.
Attachment #763497 - Flags: review?(dflanagan) → review?(dale)
Comment on attachment 763497 [details]
handle error codes for video element

Tested this and works well, I do prefer proper error handling than caring about where the video comes from
Attachment #763497 - Flags: review?(dale) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Uplifted 447c6f665d0a2908a9b0b88f575e8c2233887dfb to:
v1-train: ee63527be6592267d1bea6a82252df9da67e3cf1
1.1hd: ee63527be6592267d1bea6a82252df9da67e3cf1
You need to log in before you can comment on or make changes to this bug.