Closed Bug 959108 Opened 10 years ago Closed 10 years ago

Helper app opens different video on


(Firefox for Android Graveyard :: General, defect)

Not set


(firefox26 affected, firefox27 affected, firefox28 affected, firefox29 affected, fennec+)

Firefox 30
Tracking Status
firefox26 --- affected
firefox27 --- affected
firefox28 --- affected
firefox29 --- affected
fennec + ---


(Reporter: cos_flaviu, Assigned: esawin)




(1 file, 2 obsolete files)

Device: Asus Transformer Tab (Android 4.0.3);
Build: Nightly 29.0a1 (2014-01-12).

Steps to reproduce:
1. Go to;
2. Tap on any video from the list;
3. Tap on the video to launch the youtube app;
4. Tap on the android back button to return to firefox;
5. Tap on the android back button to go back to the main page of the;
6. Tap on a different video from the list;
7. Tap on the helper app icon in the url bar.

Expected result:
The video from the step 6 is started in the youtube app.

Actual result:
The youtube app is launched with the video from the step 3.
tracking-fennec: --- → ?
tracking-fennec: ? → +
Assignee: nobody → esawin
Currently, the page actions for helper apps are updated on |pageshow| events, which are not triggered for dynamic in-document content loads, even if the URL changes.

Moving the page action updates into the |onLocationChange| handler solves this.

It's possibly unrelated, but going back in history to the front page and clicking the helper app link opens the YouTube app and additionally launches Chrome for in my case.

1. Apply patch.
2. Goto (loads and redirects to
3. Go back using browser or android button (loads
4. Click on helper app icon in the URL bar.

Expected result:
YouTube app launches.

Actual result:
YouTube app launches and Google Chrome launches on
Attachment #8380965 - Flags: review?(
Comment on attachment 8380965 [details] [diff] [review]
Update page action on location change

Redirecting review to wesj as he's more familiar with this code.
Attachment #8380965 - Flags: review?( → review?(wjohnston)
Comment on attachment 8380965 [details] [diff] [review]
Update page action on location change

Review of attachment 8380965 [details] [diff] [review]:

Location change events will probably happen during page load. We don't want to slow it down with this query, so we'll need to find some other way to do this thats a bit more careful.
Attachment #8380965 - Flags: review?(wjohnston)
With this patch, we just update the page action URL on location change without querying for helper apps, which happens, as before, on |pageshow|.

That way, the page action  will always launch the selected apps for the current URL.

There is an unrelated issue with the intent filtering for YouTube, qualifies for the YouTube app, while does not. Since YouTube redirects to /home, |pageshow| is never fired for its root domain. The effect is, that the helper app icon only appears after clicking on a video.
Attachment #8380965 - Attachment is obsolete: true
Attachment #8381677 - Flags: review?(wjohnston)
Comment on attachment 8381677 [details] [diff] [review]
Update page action URL on location change

Review of attachment 8381677 [details] [diff] [review]:

I like this :) Can we clean up the clickCallback a little while we're here, and this will be good. Thanks :)

::: mobile/android/chrome/content/browser.js
@@ +7909,5 @@
> +  },
> +
> +  getPageActionUri: function getPageActionUri() {
> +    return this._pageActionUri;
> +  },

If all these do is get/set a value, I'd just access it directly.

@@ +7923,5 @@
>        title: Strings.browser.GetStringFromName("openInApp.pageAction"),
>        icon: "drawable://icon_openinapp",
>        clickCallback: (function() {
>          let callback = function(app) {
> +          app.launch(ExternalApps.getPageActionUri());

We can be trickier here and use fat-arrow functions to help. Also I hate this inner callback function (that I added). i.e. change this to:

clickCallback: () => {
  if (apps.length > 1) {
      // pick an app...
  } else {
Attachment #8381677 - Flags: review?(wjohnston) → feedback+
Updated patch according to review feedback.

Patch for check-in.
Attachment #8381677 - Attachment is obsolete: true
Attachment #8382528 - Flags: review+
Attachment #8382528 - Flags: review+ → review?(wjohnston)
Attachment #8382528 - Flags: review?(wjohnston) → review+
Keywords: checkin-needed
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 30
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.