1393699 - Include Pocket referer in Top Stories

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement, P1)

Description

[Chenxia Liu [:liuche]]

Splitting this out from bug 1380808.

From https://bugzilla.mozilla.org/show_bug.cgi?id=1380808#c31 : 

There are a couple of ways off the top of my head to include the referer link:
- The Pocket API includes a Pocket link that adds the referer, but this causes a redirect and adds an extra history entry
- Send the referer ourselves when loading the link - http://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1178

Comment 1

[Chenxia Liu [:liuche]]

Picking this up bc all other bugs in the sprint are assigned/blocked.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Spoke IRL, I'm taking this over.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Nate, do you happen to know if there is a standard for referrer uris in Android intent URIs (i.e. opening another Android app via a link)? For example, if a user clicks a Pocket top story that links to Medium and the user has the Medium app installed, we'd open in the Medium app. However, I'm not sure how we'd pass in the referrer URI.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

FYI: I added bug 1395409 to write a test for this.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1393699: Rm unused BrowserApp.openUrlAndStopEditing method.

Review commit: https://reviewboard.mozilla.org/r/174714/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/174714/

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1393699: Add referrerURI to Tabs.loadUrl flows.

This will allow us to call Tabs.loadUrl with a referrer URI from the Pocket top
stories.

Review commit: https://reviewboard.mozilla.org/r/174716/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/174716/

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1393699: Add Pocket referrer URI to AS top story clicks.

I tested this by replacing the placeholder top story URLs with [1], clicking on
the links, and verifying clicks from top stories included the referrer but
clicks from highlights and top sites did not.

[1]: https://www.whatismybrowser.com/detect/what-http-headers-is-my-browser-sending

Review commit: https://reviewboard.mozilla.org/r/174718/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/174718/

Comment 8

[Chenxia Liu [:liuche]]

Comment on attachment 8902957 [details]
Bug 1393699: Rm unused BrowserApp.openUrlAndStopEditing method.

https://reviewboard.mozilla.org/r/174714/#review179794

Comment 9

[Chenxia Liu [:liuche]]

Comment on attachment 8902958 [details]
Bug 1393699: Add referrerURI to Tabs.loadUrl flows.

https://reviewboard.mozilla.org/r/174716/#review179796

Glad this is a simple fix to just add a referer field. It's not super scalable, but this isn't actually used in many places so that should be okay.

Comment 10

[Chenxia Liu [:liuche]]

Comment on attachment 8902959 [details]
Bug 1393699: Add Pocket referrer URI to AS top story clicks.

https://reviewboard.mozilla.org/r/174718/#review179802

I wonder if there's anything we need to follow up with Pocket about, to make sure they see increased metrics through their referer id being used on Android?

I'm glad this solution uses Gecko's existing functionality for the referer header! Much cleaner than a redirect :)

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Follow-up TODO (maybe split into separate bugs):
- via Chenxia: find out if we need to follow up with Pocket, to make sure they see increased metrics through their referer id being used on Android?
- Android intent URIs
- Test for referrer (bug 1395409)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4eb5a8e5cb5d
Rm unused BrowserApp.openUrlAndStopEditing method. r=liuche
https://hg.mozilla.org/integration/autoland/rev/f9bcc1e12a01
Add referrerURI to Tabs.loadUrl flows. r=liuche
https://hg.mozilla.org/integration/autoland/rev/8a87c00279d7
Add Pocket referrer URI to AS top story clicks. r=liuche

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

NI self comment 11.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4eb5a8e5cb5d
https://hg.mozilla.org/mozilla-central/rev/f9bcc1e12a01
https://hg.mozilla.org/mozilla-central/rev/8a87c00279d7

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #3)
> Nate, do you happen to know if there is a standard for referrer uris in
> Android intent URIs (i.e. opening another Android app via a link)? For
> example, if a user clicks a Pocket top story that links to Medium and the
> user has the Medium app installed, we'd open in the Medium app. However, I'm
> not sure how we'd pass in the referrer URI.

Moved to bug 1396116.

(In reply to Michael Comella (:mcomella) from comment #11)
> Follow-up TODO (maybe split into separate bugs):
> - Android intent URIs

As per above, bug 1396116.

> - via Chenxia: find out if we need to follow up with Pocket, to make sure
> they see increased metrics through their referer id being used on Android?

bug 1396124.