Closed Bug 869019 Opened 7 years ago Closed 7 years ago

Regression: YouTube broken on Nightly - java.lang.StackOverflowError @ getOpenURIIntent(GeckoAppShell.java:1058)

Categories

(Firefox for Android :: General, defect, major)

ARM
Android
defect
Not set
major

Tracking

()

VERIFIED FIXED
Firefox 23
Tracking Status
firefox22 --- unaffected
firefox23 --- verified

People

(Reporter: aaronmt, Assigned: rnewman)

References

()

Details

(Keywords: regression, reproducible)

Attachments

(2 files, 1 obsolete file)

E/GeckoConsole(11822): [JavaScript Error: "NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDOMLocation.href]" {file: "http://m.youtube.com/?reload=5&rdm=mmaekim6#/watch?feature=g-logo-xit&v=-nYSwuhsLa0" line: 173}]


W/System.err(11822): java.lang.StackOverflowError
W/System.err(11822): 	at android.net.Uri.<init>(Uri.java:50)
W/System.err(11822): 	at android.net.Uri$AbstractHierarchicalUri.<init>(Uri.java:1029)
W/System.err(11822): 	at android.net.Uri$AbstractHierarchicalUri.<init>(Uri.java:1029)
W/System.err(11822): 	at android.net.Uri$StringUri.<init>(Uri.java:465)
W/System.err(11822): 	at android.net.Uri$StringUri.<init>(Uri.java:457)
W/System.err(11822): 	at android.net.Uri.parse(Uri.java:429)
W/System.err(11822): 	at org.mozilla.gecko.GeckoAppShell.getOpenURIIntent(GeckoAppShell.java:1058)

......

STR: http://m.youtube.com, tap play on one of the top-level videos.

--
Nightly (05/06)
LG Nexus 4 (Android 4.2.2)
Severity: normal → major
cpeterson broke this in Bug 847839 (landed in 22), with this chunk of (merge?) diff:

diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java
--- a/mobile/android/base/GeckoAppShell.java
+++ b/mobile/android/base/GeckoAppShell.java
@@ -1055,17 +1055,26 @@ public class GeckoAppShell
             return intent;
         }

         if (!isUriSafeForScheme(uri)) {
             return null;
         }

         final String scheme = uri.getScheme();
-        final Intent intent = getIntentForActionString(action);
+
+        final Intent intent;
+        if ("vnd.youtube".equals(scheme) && getHandlersForURL(targetURI, action).length == 0) {
+            // Special case youtube to fallback to our own player
+            intent = new Intent(VideoPlayer.VIDEO_ACTION);
+            intent.setClassName(GeckoApp.mAppContext.getPackageName(),
+                                "org.mozilla.gecko.VideoPlayer");
+        } else {
+            intent = getIntentForActionString(action);
+        }
I think the fix here is to remove the recursive call, and replace it with a direct call to getHandlersForIntent with the fallback intent.
I'll have a patch up soon, I hope, but someone else might have to pick this up and finish it, because I'm not 100% sure what the correct behavior is supposed to be.
Blocks: 847839
Attached patch Tentative patch. v1 (obsolete) — Splinter Review
Let's see how this is. (It's still building and untested, but...)
Attachment #745901 - Flags: review?(cpeterson)
Now with fewer syntax errors!
Attachment #745901 - Attachment is obsolete: true
Attachment #745901 - Flags: review?(cpeterson)
Attachment #745902 - Flags: review?(cpeterson)
Comment on attachment 745902 [details] [diff] [review]
Tentative patch. v2

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

LGTM. Thanks for fixing my regression! <:)
Attachment #745902 - Flags: review?(cpeterson) → review+
Flags: in-testsuite?
Waiting for Fennec to stop crashing on my phone so I can test this…
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/4f95c172c51d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
can we get a functionality test for getting an external helper application?
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.