Tested in Beta and Nightly, and no YouTube video will play. Doesn't crash, but only get a blank screen. Need to get some logs for this; needinfo for myself.

For those following along at home, the Kindle's don't have a YouTube player app, so we ship our own barebones VideoPlayer activity. It tries to parse out the video stream from the YouTube URL.

What happened to VideoPlayer.java (reminisce down bug 847839 lane via patch 1a)?

My hypothesis is that it stopped working (or never worked), because we almost exclusively all run devices with YouTube players and Flash. Either that, or I regressed something in the stonkingly simple patch in Bug 886014. We'll see when I sit down tomorrow and get a log...

exception android.os.NetworkOnMainThreadException at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java:1117) at java.net.InetAddress.lookupHostByName(InetAddress.java:385) at java.net.InetAddress.getAllByNameImpl(InetAddress.java:236) at java.net.InetAddress.getAllByName(InetAddress.java:214) at libcore.net.http.HttpConnection.<init>(HttpConnection.java:70) at libcore.net.http.HttpConnection.<init>(HttpConnection.java:51) at libcore.net.http.HttpConnection$Address.connect(HttpConnection.java:359) at libcore.net.http.HttpConnectionPool.get(HttpConnectionPool.java:86) at libcore.net.http.HttpConnection.connect(HttpConnection.java:128) at libcore.net.http.HttpEngine.openSocketConnection(HttpEngine.java:316) at libcore.net.http.HttpEngine.connect(HttpEngine.java:311) at libcore.net.http.HttpEngine.sendSocketRequest(HttpEngine.java:290) at libcore.net.http.HttpEngine.sendRequest(HttpEngine.java:240) at libcore.net.http.HttpURLConnectionImpl.getResponse(HttpURLConnectionImpl.java:282) at libcore.net.http.HttpURLConnectionImpl.getInputStream(HttpURLConnectionImpl.java:177) at org.mozilla.gecko.VideoPlayer.getSpecFromYouTubeVideoID(VideoPlayer.java:69) at org.mozilla.gecko.VideoPlayer.onCreate(VideoPlayer.java:50)

It turns out that VideoPlayer is borked for more than just StrictMode violations. I fixed that, and now it seems like YouTube has changed their URI format. So I fixed that. And now the player UI loads some of the video, doesn't seem to play anything, and doesn't correctly respond to rotations. Here's a patch, but we're not done yet. mfinkle gets review, 'cos he's the only person in blame who's still on the team!

Note that until we finish this bug (i.e., figure out why videos aren't playing), I suggest we don't ship VideoPlayer at all, and instead fire a VIEW intent to let the system browser or other apps try. People do sideload Firefox onto these devices, so continuing to ship something totally broken seems like a bad choice.

Also need to check this on an AOSP device other than the Kindle, see if it's just as borked there.

Comment on attachment 831239 [details] [diff] [review] Part 1: initial fixes. v1 The only part is was wondering about is the ThreadUtils part. GeckoApp sets some of the ThreadUtils data members, IIRC. Are we OK to be using ThreadUtils from VideoPlayer? I guess those statics would have been setup since VideoPlayer is always launched from Fennec, which initializes them.

(In reply to Mark Finkle (:mfinkle) from comment #8) > I guess those statics would have been setup since VideoPlayer is always > launched from Fennec, which initializes them. The only thing that wouldn't be set is sUiThread -- the background thread will be created by GeckoBackgroundThread if needed. But yes, so long as GeckoApp runs first, we should be fine here.

Proposition: we should throw away VideoPlayer and ship http://code.google.com/p/android-youtube-player/wiki/OpenYouTubePlayerActiviyInstructions Alternatively, we should investigate whether playing the content from the mobile version of youtube.com is satisfactory. Thoughts?

(In reply to Richard Newman [:rnewman] from comment #10) > Proposition: we should throw away VideoPlayer and ship > > http://code.google.com/p/android-youtube-player/wiki/ > OpenYouTubePlayerActiviyInstructions What makes you think this is going to be maintained? > Alternatively, we should investigate whether playing the content from the > mobile version of youtube.com is satisfactory. Requesting the HTML5 video version of YouTube would be more of a product call. Previously we've dcome the conclusion that the YouTube App is a better experience (and available to the vase majority of our users). But... One solution may be to, when there is no handler for vnd-youtube, reload the page in a way to get the HTML5 version (I think you can do something like http://www.youtube.com/embed/<vidoe ID>?html5=1) That said, not all videos will be available in HTML5, you know DRM and all that. > > Thoughts?

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #11) > What makes you think this is going to be maintained? It's going to be maintained at least as much as VideoPlayer is. > But... One solution may be to, when there is no handler for vnd-youtube, > reload the page in a way to get the HTML5 version (I think you can do > something like http://www.youtube.com/embed/<vidoe ID>?html5=1) Yeah, that's the situation in which I'm suggesting this. Essentially, if you're on a device without a YouTube player, we should try playing the video inside the browser, rather than launching a separate activity, given that the separate activity doesn't actually seem to work. > That said, not all videos will be available in HTML5, you know DRM and all > that. But that's much the same situation we have with VideoPlayer, right? -- sometimes the info request comes back with "This video isn't available due to...", or just no stream. I had a one-in-three success rate when I was trying to find a video to test against.

I agree with the plan you two are forming: 1. Use vnd-youtube to spawn out to the YouTube player 2. If not available, reload to get the HTML5 version and play it in the browser 3. If no HTML5 version, we apologize I think we still send the XUL Fennec UA to YouTube so we get vnd-youtube streams. Disabling that UA override should get the HTML5 version. It works for embedded YouTube videos.

(In reply to Mark Finkle (:mfinkle) from comment #13) > I agree with the plan you two are forming: > 1. Use vnd-youtube to spawn out to the YouTube player > 2. If not available, reload to get the HTML5 version and play it in the > browser > 3. If no HTML5 version, we apologize > > I think we still send the XUL Fennec UA to YouTube so we get vnd-youtube > streams. Disabling that UA override should get the HTML5 version. It works > for embedded YouTube videos. going a step further, if there is no YouTube player available, set a pref to override the UA override in general so we only have to do this dance once

To avoid further confusion (see bug 906869 comment 23) would it make sense to adjust the description of this bug?

I'm working on this. The embedding video player is fullscreen, lacks comments, etc; we either need to find a different invocation (such as loading without a special UA), or continue using the action icon in the URL bar to launch the 'player'. More as I test.

Tapping a YouTube link: 07-30 12:46:44.397 I/GeckoAppShell(22948): getHandlersForURL: vnd.youtube, 07-30 12:46:44.437 I/GeckoAppShell(22948): getHandlersForURL: vnd.youtube, 07-30 12:46:44.437 I/GeckoAppShell(22948): openUriExternal: vnd.youtube:SCLU3v_AY4Q?vndapp=youtube_mobile&vndclient=mv-google&vndel=watch&vnddnc=1

This stubs out the real integration point; see Part 2.

This works for me. Loading a YouTube video page gives you the usual clickable video. Tapping that opens the HTML player rather than opening the YouTube app; you can watch the video then hit Back to return to the video page. Really nice.

Comment on attachment 8465050 [details] [diff] [review] Part 2: delegate to Fennec itself for YouTube HTML5 video instead of VideoPlayer. v1 >diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java > static String[] getHandlersForURL(String aURL, String aAction) { >+ Log.v(LOGTAG, "getHandlersForURL: " + aURL + ", " + aAction); Don't leak URIs. Let's just remove it. > public static boolean openUriExternal(String targetURI, > String mimeType, > @OptionalGeneratedParameter String packageName, > @OptionalGeneratedParameter String className, > @OptionalGeneratedParameter String action, > @OptionalGeneratedParameter String title) { >+ Log.v(LOGTAG, "openUriExternal: " + targetURI); >+ Don't leak URIs. Let's just remove it. > public static Intent getShareIntent(final Context context, > final String targetURI, > final String mimeType, > final String title) { >+ Log.v(LOGTAG, "getShareIntent: " + targetURI); >+ Don't leak URIs. Let's just remove it. >+ if (youtubeURI != null) { >+ Log.v(LOGTAG, "Opening YouTube URI: " + youtubeURI); Don't leak URIs. Let's just remove it. >+ private static Uri getYouTubeHTML5URI(final Uri uri) { >+ if (uri == null) { >+ return null; >+ } >+ >+ Log.v(LOGTAG, "Incoming URI: " + uri); Don't leak URIs. Let's just remove it.

https://hg.mozilla.org/integration/fx-team/rev/2c54d1bdd339 https://hg.mozilla.org/integration/fx-team/rev/4cbef502c1dc

Relnote: "Fix YouTube playback on devices without YouTube app." Assuming this verifies fine, I'd want this for 33. Any contrary opinions? mfinkle?

https://hg.mozilla.org/mozilla-central/rev/2c54d1bdd339 https://hg.mozilla.org/mozilla-central/rev/4cbef502c1dc

> # pm disable com.google.android.youtube * Played video directly on YouTube on my Nexus 5 (4.4.4) * Embedded YouTube works

Comment on attachment 8465050 [details] [diff] [review] Part 2: delegate to Fennec itself for YouTube HTML5 video instead of VideoPlayer. v1 Approval Request Comment [Feature/regressing bug #]: Forever. [User impact if declined]: Crashes when attempting to play YouTube videos on AOSP/Kindle Fire. [Describe test coverage new/current, TBPL]: Manually verified. [Risks and why]: We're loosely targeting 33 as our Kindle-supporting release, and this is actually quite serious for non-Google-branded users, too. [String/UUID change made/needed]: None.

(In reply to Richard Newman [:rnewman] from comment #23) > Relnote: "Fix YouTube playback on devices without YouTube app." It's a subpar experience in comparison to using the native YouTube application in entirety, not sure how you would describe that.

(In reply to Aaron Train [:aaronmt] from comment #27) > It's a subpar experience in comparison to using the native YouTube > application in entirety, not sure how you would describe that. I think it's implied that we can't send you to the YouTube app… we don't suggest "reimplemented the entire YouTube experience". Beyond that, I actually feel that the experience is pretty good! It does fullscreen, and even does casting, I think.

(In reply to Richard Newman [:rnewman] from comment #28) > (In reply to Aaron Train [:aaronmt] from comment #27) > > > It's a subpar experience in comparison to using the native YouTube > > application in entirety, not sure how you would describe that. > > I think it's implied that we can't send you to the YouTube app… we don't > suggest "reimplemented the entire YouTube experience". > > Beyond that, I actually feel that the experience is pretty good! It does > fullscreen, and even does casting, I think. Agreed. It's pretty good.

http://hg.mozilla.org/releases/mozilla-aurora/rev/ae5bc80c1b9e http://hg.mozilla.org/releases/mozilla-aurora/rev/3dc4eb0e81c6

Comment on attachment 8465050 [details] [diff] [review] Part 2: delegate to Fennec itself for YouTube HTML5 video instead of VideoPlayer. v1 Mark and I discussed getting this into Beta. It's a safe change: we're happy with it in Nightly and on Aurora. It addresses an obvious and painful crash for not only Kindle but also other AOSP users, which is a pretty big market (in some locales, bigger than Google-licensed Android; cf Xiaomi). And the change only affects those users: even if this is buggy, it can't be worse than our current "just crash" behavior.

Verified as fixed in: Build: Firefox for Android 33.0a2 (2014-08-11) Device: Kindle Fire HD 7"

Comment on attachment 8465050 [details] [diff] [review] Part 2: delegate to Fennec itself for YouTube HTML5 video instead of VideoPlayer. v1 Ouch that we need to have more site specific code in Fennec. I agree with the risk assessment and this it is worth trying to push this out sooner given the limited impact that it should have on the general Fennec population. Let's get this into beta6. beta+

https://hg.mozilla.org/releases/mozilla-beta/rev/e9fc6b3efaf3 https://hg.mozilla.org/releases/mozilla-beta/rev/086068fe1b98

Verified as fixed in: Build: Firefox for Android 32 Beta 6 Device: Kindle Fire HD 7"