All users were logged out of Bugzilla on October 13th, 2018

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

VERIFIED FIXED in Firefox 23

Status

()

--
major
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: aaronmt, Assigned: rnewman)

Tracking

({regression, reproducible})

Trunk
Firefox 23
ARM
Android
regression, reproducible
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox22 unaffected, firefox23 verified)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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)
(Reporter)

Updated

6 years ago
Severity: normal → major
(Reporter)

Comment 1

6 years ago
Created attachment 745876 [details]
Raw log (Nightly 05/06)
(Assignee)

Comment 3

6 years ago
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);
+        }
(Assignee)

Updated

6 years ago
status-firefox22: --- → affected
(Assignee)

Comment 4

6 years ago
I think the fix here is to remove the recursive call, and replace it with a direct call to getHandlersForIntent with the fallback intent.
(Reporter)

Updated

6 years ago
status-firefox22: affected → unaffected
(Assignee)

Comment 5

6 years ago
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
(Assignee)

Comment 6

6 years ago
Created attachment 745901 [details] [diff] [review]
Tentative patch. v1

Let's see how this is. (It's still building and untested, but...)
Attachment #745901 - Flags: review?(cpeterson)
(Assignee)

Comment 7

6 years ago
Created attachment 745902 [details] [diff] [review]
Tentative patch. v2

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+
(Reporter)

Updated

6 years ago
Flags: in-testsuite?
(Assignee)

Comment 9

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
can we get a functionality test for getting an external helper application?

Updated

6 years ago
status-firefox23: affected → fixed
(Reporter)

Updated

6 years ago
Status: RESOLVED → VERIFIED
status-firefox23: fixed → verified
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.