Unable to play videos on mobile YouTube; redirection loop on 'Play'

VERIFIED FIXED

Status

()

Firefox for Android
General
--
major
VERIFIED FIXED
5 years ago
9 months ago

People

(Reporter: csuciu, Assigned: mfinkle)

Tracking

(Blocks: 1 bug)

16 Branch
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox14 verified, firefox15 fixed, firefox16 verified, blocking-fennec1.0 betaN+)

Details

(URL)

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 635258 [details]
Log file

Nightly 16.0a1 (2012-06-21)

Steps:
1. Go to m.youtube.com
2. Tap on one of the video
3. Tap on play button for the selected video

Expected
Video starts to play

Actual:
Page is refreshed

Log attached

Updated

5 years ago
blocking-fennec1.0: --- → ?
status-firefox14: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → affected
So YouTube's changes worked late afternoon yesterday. On 'play' one would be redirected to their desktop site. It looks like they changed something and it's in a user-agent redirection loop.

Updated

5 years ago
Severity: normal → major

Updated

5 years ago
blocking-fennec1.0: ? → +
As commented in an e-mail thread, spoofing the iPhone UA gets us to the point where we have an unplayable video. There is a different path (user-agent redirection loop as Aaron points out above) for Fennec.

Updated

5 years ago
Assignee: nobody → blassey.bugs

Updated

5 years ago
Summary: Page refreshed on m.youtube.com when tapping on 'play video' button → Unable to play videos on mobile YouTube; redirection loop on 'Play'
Duplicate of this bug: 766855
Blocks: 759982
A couple notes:

1) XUL Fennec works fine, kicks out to the youtube app on the device as it always has
2) Native Fennec with its UA switched to XUL Fennec's also works fine, kicking out to the youtube app

if we could get google to serve native fennec the same content as xul fennec, that would be ideal
cc Jet, who was in contact with YouTube about getting mobile content in Firefox.
And kev, on the off chance he was on the same thread, and wakes up earlier.

Updated

5 years ago
blocking-kilimanjaro: --- → ?
(In reply to Brad Lassey [:blassey] from comment #4)
> A couple notes:
> 
> 1) XUL Fennec works fine, kicks out to the youtube app on the device as it
> always has
> 2) Native Fennec with its UA switched to XUL Fennec's also works fine,
> kicking out to the youtube app
> 
> if we could get google to serve native fennec the same content as xul
> fennec, that would be ideal

Note that the YouTube player app needs to be installed for the "kick out" to work.
Created attachment 635315 [details] [diff] [review]
patch

This patch uses the basics of the "Request Desktop Site" patch to force sending a XUL UA to youtube.  The domain matching is a bit loose, only testing for "youtube" anywhere in the host.
Attachment #635315 - Flags: review?(bnicholson)
Created attachment 635316 [details] [diff] [review]
patch to send XUL Fennec UA to yotuube
Attachment #635316 - Flags: review?(mark.finkle)
FYI - This bug is on the top list of topics to talk about at the mobile web compat meeting at 9am, if anyone is interested in attending to discuss it.
(Assignee)

Updated

5 years ago
Attachment #635315 - Flags: review?(blassey.bugs)
(Assignee)

Updated

5 years ago
Attachment #635316 - Attachment is obsolete: true
Attachment #635316 - Flags: review?(mark.finkle)
Comment on attachment 635315 [details] [diff] [review]
patch

Review of attachment 635315 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/chrome/content/browser.js
@@ +2868,5 @@
> +#expand     let version = "__MOZ_APP_VERSION__";
> +            ua += " Fennec/" + version;
> +            channel.setRequestHeader("User-Agent", ua, false);
> +          }
> +        }

nit, enclose this case in curly brackets so channel and channelWindow are only scoped to this case
Attachment #635315 - Flags: review?(blassey.bugs) → review+
(Assignee)

Updated

5 years ago
Attachment #635315 - Flags: review?(bnicholson) → review?(mbrubeck)
As an example, in XUL Fennec when you tap the video to start playing, Youtube sends us this URL:

vnd.youtube:e9flNIz9qhg?vndapp=youtube_mobile&vndclient=mv-google&vndel=home&vnddnc=1
Question: If you tube revs their site again, will this fix over-ride their changes?
blocking-kilimanjaro: ? → ---
Attachment #635315 - Flags: review?(mbrubeck) → review+
Comment on attachment 635315 [details] [diff] [review]
patch

>+    Services.obs.removeObserver(this, "http-on-modify-request", false);

Nit: The last argument (false) should be removed.
One thing to consider is "http-on-modify-request" is called for every resource on every page. Having N tabs open would multiply the number of observed callbacks by N. It would be more efficient to have a single observer that operates on all tabs rather than having each tab listen for this event. I was going to make this same change in bug 766406.

Maybe you could put this in a class called UserAgent, and I could merge my AgentMode class into that?
(In reply to Brian Nicholson (:bnicholson) from comment #15)
> One thing to consider is "http-on-modify-request" is called for every
> resource on every page. Having N tabs open would multiply the number of
> observed callbacks by N. It would be more efficient to have a single
> observer that operates on all tabs rather than having each tab listen for
> this event. I was going to make this same change in bug 766406.
> 
> Maybe you could put this in a class called UserAgent, and I could merge my
> AgentMode class into that?

Good point.  For simplicity in this patch (which is a temporary hack and will be removed soon), we could just add the observer to BrowserApp instead of Tab.prototype.
Also, it may not be a big deal, but this check would match other hosts containing "youtube" (youtube-global.blogspot.com, www.youtube-mp3.org). You could make sure the end of the host string is ".youtube.com" to avoid this.
(In reply to Brian Nicholson (:bnicholson) from comment #17)
> Also, it may not be a big deal, but this check would match other hosts
> containing "youtube" (youtube-global.blogspot.com, www.youtube-mp3.org). You
> could make sure the end of the host string is ".youtube.com" to avoid this.

Although that wouldn't match http://youtube.com - maybe a regex like:
/^(.+\.)?youtube\.com$/ ?
(In reply to Mark Finkle (:mfinkle) from comment #8)
> Created attachment 635315 [details] [diff] [review]
> patch
> 
> This patch uses the basics of the "Request Desktop Site" patch to force
> sending a XUL UA to youtube.  The domain matching is a bit loose, only
> testing for "youtube" anywhere in the host.

Oh, just saw this.
Created attachment 635365 [details] [diff] [review]
patch 2

This patch moves the code to a singleton. It was just as easy as doing it in BrowserApp and a tab cleaner imo.
Assignee: blassey.bugs → mark.finkle
Attachment #635315 - Attachment is obsolete: true
Attachment #635365 - Flags: review?(blassey.bugs)
Attachment #635365 - Flags: review?(blassey.bugs) → review+
Comment on attachment 635365 [details] [diff] [review]
patch 2

>+  uninit: function ua_uninit() {
>+    Services.obs.removeObserver(this, "http-on-modify-request", false);
>+  },

Same nit as before: The "false" should be removed.
Attachment #635365 - Flags: approval-mozilla-beta+
Attachment #635365 - Flags: approval-mozilla-aurora+
Created attachment 635373 [details] [diff] [review]
patch for checkin

Patch with Matt's nit fixed. Ready to push.
Attachment #635365 - Attachment is obsolete: true
Attachment #635373 - Flags: review+
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox14: affected → fixed
status-firefox15: affected → fixed
status-firefox16: affected → fixed
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
blocking-fennec1.0: + → betaN+
Top priority for QA to verify.
Whiteboard: [qa+]
https://hg.mozilla.org/mozilla-central/rev/551d99cd89d4
https://hg.mozilla.org/releases/mozilla-aurora/rev/01e96c76e268
https://hg.mozilla.org/releases/mozilla-beta/rev/f54b2bbd3baf (14_0_Beta)
https://hg.mozilla.org/releases/mozilla-beta/rev/35a98282d2a6 (MOBILE140_2012061216_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-beta/rev/aea5ee5b6801 (default)
Created attachment 635404 [details] [diff] [review]
alternate patch that sends desktop UA

This patch applies over that previous patch and sends the desktop UA instead of XUL UA.
(Assignee)

Updated

5 years ago
Attachment #635404 - Flags: feedback?(bnicholson)
Attachment #635404 - Flags: feedback?(bnicholson) → feedback+
Tested the private build in channel with the first patch checked in and playback functionality was restored; YouTube is fully functional as a mobile site now (rather pleasant to use now over desktop). I have not encountered any bizarre regressions thus far. 

Kevin, Naoki, what's it like on 2.2?

--
Tested via Galaxy Nexus (Android 4.0.4) & Droid Bionic (Android 2.3)
Comment on attachment 635373 [details] [diff] [review]
patch for checkin

>+      if (channel.URI.host.indexOf("youtube") != -1) {

I think you probably want a stricter check for whether this is the host you want.  (Probably either an equality check or maybe something using the effective-TLD code?)
(In reply to David Baron [:dbaron] from comment #27)
> I think you probably want a stricter check for whether this is the host you
> want.  (Probably either an equality check or maybe something using the
> effective-TLD code?)

We decided that false positives were basically harmless for this temporary hack, so we intentionally erred on that side.

Comment 29

5 years ago
(In reply to Aaron Train [:aaronmt] from comment #26)
> Tested the private build in channel with the first patch checked in and
> playback functionality was restored; YouTube is fully functional as a mobile
> site now (rather pleasant to use now over desktop). I have not encountered
> any bizarre regressions thus far. 
> 
> Kevin, Naoki, what's it like on 2.2?
> 
> --
> Tested via Galaxy Nexus (Android 4.0.4) & Droid Bionic (Android 2.3)

Tested on Droid 2, android 2.2, the fix is working.
Verified Fixed via 14.0 Beta 8 - Candidate #1, Build #2 (buildID=20120621141442)

--
Samsung Galaxy Nexus (Android 4.0.4)
Samsung Nexus S (Android 4.0.3)
HTC Nexus One (Android 2.3.4)
Status: RESOLVED → VERIFIED
status-firefox14: fixed → verified

Updated

5 years ago
Duplicate of this bug: 767257

Comment 32

5 years ago
Reopening this one per this July 5 e-mail from youtube:

"We verified that our fix works with both the stable release that launched last week, as well as the "Firefox Beta" app in the store. It is live now, so you can verify that it works on m.youtube.com"

They pushed a fix that should render the UA hack unnecessary. I'd love to back it out of Fennec if the youtube server-side fix is verified. Testing a Fennec build from just before 14.0b8 should verify the server changes.
Status: VERIFIED → REOPENED
Keywords: qawanted
Resolution: FIXED → ---
As a point of comparison, testing this on Firefox OS that does not contain the hack ends up showing vnd.youtube when you try to view a video (I believe that's the youtube android activity).
Tested using FF Android 14.0 Beta 6 - Video plays with no issues.

I might suggest though we open a separate bug to track this to eliminate possible confusion.
Keywords: qawanted

Updated

5 years ago
tracking-fennec: --- → ?
Aaron - Can you close/verify this bug and open a new bug to backout the change?
(In reply to Mark Finkle (:mfinkle) from comment #35)
> Aaron - Can you close/verify this bug and open a new bug to backout the
> change?

bug 778561
Status: REOPENED → RESOLVED
tracking-fennec: ? → ---
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Whiteboard: [qa+]

Updated

5 years ago
Status: RESOLVED → VERIFIED
It would appear that this is happening again - Going to any Youube video on the mobile site in current Nightly results in a refresh loop on play.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(In reply to Chris Lord [:cwiiis] from comment #37)
> It would appear that this is happening again - Going to any Youube video on
> the mobile site in current Nightly results in a refresh loop on play.

I don't encounter this issue with the current nightly. YouTube videos correctly kick out to the YouTube app. I tested with Nightly 2012-08-25 on a Samsung Galaxy S2 ICS 4.0.4. With what version of Nightly and what device and OS are you testing?
Nightly 2012-08-25 on a Google Nexus 7.
:cwiiis - 
1. What version of Android are you running? 
2. Can you reproduce the issue on all channels or only on Nightly? 
3. Have you tried clearing the data/cache for Nightly? Any change?
08/27 GN 4.1.1, I get the Android YouTube player; wfm.
Closing.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(In reply to Jason Smith [:jsmith] from comment #42)
> Closing.

This still doesn't work on the Nexus 7. Will file a new bug as people seem eager to keep this one closed.

Updated

5 years ago
Depends on: 786623

Updated

5 years ago
Status: RESOLVED → VERIFIED
status-firefox16: fixed → verified
You need to log in before you can comment on or make changes to this bug.