Closed Bug 821514 Opened 12 years ago Closed 12 years ago

Video from is not playing


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

Gonk (Firefox OS)


(blocking-basecamp:+, firefox19 fixed, firefox20 fixed, b2g18 fixed)

B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed


(Reporter: nhirata, Assigned: djf)



(Keywords: late-l10n, regression)


(4 files, 1 obsolete file)

1. launch browser
2. go to
3. tap on the video

Expected: video plays
Actual: nothing happens

Note: 1/13/2012 build. unagi
Triage: BB+, C3, P2 - regression
blocking-basecamp: ? → +
Keywords: regression
Priority: -- → P2
Target Milestone: --- → B2G C3 (12dec-1jan)
Please note that this issue is not a general case.
From my test, it should be OK to play some other videos on YouTube.

For this specific video,, the root cause that it could not be played back,

  1. We would try to get the video info from Gecko's b2g/components/YoutubeProtocolHandler.js with this url,

  2. In this case it would return failure, as the attachment.

We need to consider error handling for youtube video playing.
I'm working on a couple of related bugs, so I'll take this one also.

At a minimum, I'll make the video app display an error message if the video fails to play.  If time allows, I'll do something more intelligent with the get_video_info API that Rudy describes in comment 2.
Assignee: nobody → dflanagan
Actually, I'm not going to take this one. This needs to be fixed in gecko, as Rudy said, and I can't do anything about it in Gaia.

It seems clear how to detect the error, and even how to extract an error message. I just don't know enough about gecko protocol handlers to know how to report an error in a useful way.  Nor do I know what the browser would do when that error occurs.

If someone can help with that info, I'm willing to have this bug assigned to me, but there may be others who can handle it better than I can.
Assignee: dflanagan → nobody
Hmm.  Maybe I can handle this in Gaia.  If the protocol handler gets an error when calling the get_video_info API, that error needs to be displayed to the user somehow. If the browser doesn't have any good way to display this error, then we should make the activity handler that plays youtube videos display youtube-related errors.  So maybe the error message should just be passsed in the activity request.
Assignee: nobody → dflanagan
OS: Mac OS X → Gonk (Firefox OS)
Priority: P2 → P1
Hardware: x86 → ARM
Hi, David, are you still working on this? Thanks!
(In reply to khu from comment #8)
> Hi, David, are you still working on this? Thanks!

I haven't actually started on it, but plan to. I think the fix is that if youtube isn't willing to let us display the video, we pass the error message in the view activity to the video player, and have the video display the error message instead of the video.

(Unless there is some other way to get an error message from the youtube protocol handler back to the browser to be displayed in some nice way...)

Even though this is a P1, I kind of need to fix bug 815791 and bug 815826 (both P3s) before I can fix this one. That's what the hold up has been.
b2g/components/YoutubeProtocolHandler.js is kind of a mess.  It is not parsing the video title properly which explains why we sometimes get a title and sometimes do not.

I'm fixing that along with my fix to check the status code of the result from youtube. If youtube won't give us an http: URL for the video, then we'll pass the error code and reason string to the activity handler for display there.

I've got a patch that I'm in testing now.
Keywords: late-l10n
I've flagged this as late-l10n and cc'ed Stas because if youtube won't let us play the video we need to display an error message of some sort.

Youtube sends us an error message like this: "This video contains content from Sony ATV Publishing. It is restricted from playback on certain sites." I've seen reports online of this error message appearing in other languages, so hopefully it will be appropriately localized.

But we still need a title of some sort that is guaranteed to be localized. It needs to give the message "we can't play this video" and also the message "it is youtube's fault, not ours".

Maybe "Can't play video. Youtube says:"
This is the gecko-side patch for this bug.  It modifies YoutubeProtocolHandler to check the status parameter and pass any youtube error messages on to the view activity so they can be displayed to the user.

It also fixes the code to property parse the title and thumbnail images.  The parsing code is consolidated into the nested parseYoutubeVideoInfo() function.
Attachment #696442 - Flags: review?(dale)
This is the gaia-side piece of the fix: if youtubeprotocolhandler sends an error message, the video view activity will display that message (using alert() for simplicity) instead of playing a video.

See the notes on github: the pull request includes two commits. The first is for bug 815791. The second commit is for this bug and is what needs review here.
Attachment #696447 - Flags: review?(dale)
Comment on attachment 696442 [details] [diff] [review]
patch for b2g/components/YoutubeProtocolHandler.js

+    function log(msg) {
+      msg = "YoutubeProtocolHandler.js: " + (msg.join ? msg.join(" ") : msg);

Does e.message ever pass an array? either way I dont think the join logic belongs in the log function

+        if (x < y) 
+          return 1;
+        else if (y > x) 
+          return -1;
+        else 
+          return 0;

x and y are both integers here so I think |return x - y| is the idiomatic way to sort them, but not a big deal.

Both are nits so can land without, rest is good by me
Attachment #696442 - Flags: review?(dale) → review+
Blocks: 809188
Thanks for those comments... I copied the log function from ContentHandler.js or something like that, since dump() wasn't working for me.  And I don't know what I was thinking about the array sorting function. I'll fix that.

Do you know whether I need to update the UUID of this file for a change like this?
Do you mean the Components.ID? as far as I know that is only needed when creating new components so I dont think so, but whoever checks it in can confirm
Attachment #696447 - Flags: review?(stas)
Dale - it looks like you merged the gaia patch, but didn't mark r+ here.  Would you do that please?  Since this has landed, it means that youtube titles won't display at all until the gecko patch lands.

Stas - I've asked you to review the gaia patch (that was just merged) because it includes a new string as described in comment 11.  Thanks.  Hope I'm doing this late-l10n stuff right!
Dale: no, I mean the third attachment. This one:

You reviewed it on github and merged it, but didn't mark r+ here.
Attachment #696447 - Flags: review?(dale) → review+
Oh, sorry about that, done
This is a new version of the gecko patch, with a simplified sort comparator function.  Its a cosmetic change only and I've tested this new version.
Attachment #696442 - Attachment is obsolete: true
Comment on attachment 696447 [details]
link to patch on github

Grabbing the review request from stas.

AFAICT, the string should use sentence case, as it's not a button? See
Attachment #696447 - Flags: review?(stas) → review-
PS: I'm not going to migrate the string to gaia-l10n for now, so we can fix the string without key change still.
My thought was that the youtube-error-prefix string would serve as a sort of title to the error message that youtube sends us, so that is why I started each word with a capital and used two newlines to separate it from the youtube error message.  (Ideally, I'd use a more sophisticated UI than the alert() function to display the title and the message, but there isn't time.)

I'll change to lowercase and add a colon at the end so the string reads:

   YouTube won't play video:

Does that seem better?

I suppose I should open a new bug for this change, though.
Blocks: 825522
I've moved the final l10n string issue to new bug 825522
To close the discussion both here and in the follow-up:

Titles are sentence-case, so yes, the new string is better. Just buttons with actionable verbs are camel case, and should only be one or two words.
Busted. See bug 825698.
Verified that videos that Youtube does not allow to be displayed on certain devices are now giving the user the following error message, "Well, this is embarrassing. We tried to display this Web page, but it's not responding." with a Reload button underneath. 
Checked on Build 20130103070201 on 1/3/13

Actually what you describe is a symptom of bug 825698. You get that for all youtube videos right now.

You should see a different message, but you won't be able to verify until that other bug is fixed.
Returning issue to it's Resolved state until bug 825698 has been resovled.
Closed: 12 years ago12 years ago
Attached image Error msg
Please see the attachment on comment #32. Are you talking about this error msg.

On unagi Build ID: 20130113070202 v 1.0.0-Prerelease

The screenshot you've attached is verification that this bug is resolved.  You tried to play a video that youtube won't let us play. Previously there was no explaination. Now we display the youtube error to the user so they know what is going on.

The error mentioned in comments 29 and 30 was a different error, caused by a different bug, now resolved.

All is good here!
Verified On unagi Build ID: 20130113070202 v 1.0.0-Prerelease
You need to log in before you can comment on or make changes to this bug.