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

Status: RESOLVED FIXED

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

Description

[Dominic Kuo [:dkuo]]

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.

Comment 2

[Dominic Kuo [:dkuo]]

Attachment: handle error codes for video element

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.

Comment 3

[Sotaro Ikeda [:sotaro]]

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.

Comment 4

[David Flanagan [:djf]]

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 5

[David Flanagan [:djf]]

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.

Comment 6

[Sotaro Ikeda [:sotaro]]

Comment on attachment 763497 [details]
handle error codes for video element

feedback+. It is better to check which error is generated in actual device.

Comment 7

[Alex Keybl [:akeybl]]

Blocking on 880571 in support of bug 881388.

Comment 8

[Dominic Kuo [:dkuo]]

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 9

[Dominic Kuo [:dkuo]]

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.

Comment 10

[Alex Keybl [:akeybl]]

Blocks bug 881388, so this is a QE3 bug.

Comment 11

[Dominic Kuo [:dkuo]]

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.

Comment 12

[Dale Harvey (:daleharvey)]

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

Comment 13

[Dale Harvey (:daleharvey)]

Merged in: https://github.com/mozilla-b2g/gaia/commit/447c6f665d0a2908a9b0b88f575e8c2233887dfb

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 447c6f665d0a2908a9b0b88f575e8c2233887dfb to:
v1-train: ee63527be6592267d1bea6a82252df9da67e3cf1

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: ee63527be6592267d1bea6a82252df9da67e3cf1