1239513 - Replace about:feedback with page hosted on input.mozilla.org

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Justin Crawford [:hoosteeno] [:jcrawford]]

In bug 1225676 we're porting the feedback form to Input. That code[0] needs some URL parameters from the client to help make the experience work as it does now. Specifically, it needs parameters like this:

?source=feedback-prompt&channel=firefox&url=http://www.mozilla.org#moreinfo

1. source=feedback-prompt is already happening. I don't think we need to change anything to make this work. This param determines whether we treat the request like it's coming from Fennec (e.g. use intent rather than http for play store link).

2. channel=firefox or channel=firefox_beta will determine where the play store link goes. If not present, defaults to firefox.

3. url=www.foo.com is pre-filled in the "last site visited" field. If not present, nothing goes in that field. In the current fennec form the field is readonly so people cannot type a URL there; if we can guarantee this parameter will be sent I will make the field readonly in the new form too.

[0] https://github.com/mozilla/fjord/pull/723/files

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

It's not documented, but the url can take a channel, too:

https://github.com/mozilla/fjord/blob/master/fjord/feedback/urls.py#L16

i.e.:

https://input.mozilla.org/feedback/%PRODUCT%/%VERSION%/%CHANNEL%

%PRODUCT% has to be a slug that's in the product table. %VERSION% and %CHANNEL% can be anything 30 characters or less.

For "source", it's better to switch it to utm_source. That way when we deprecate "source", we're ok:

http://fjord.readthedocs.org/en/latest/feedback_urls.html#source-campaign

Hope that helps!

Comment 2

[:Margaret Leibovic]

I'll try to get to this some time next week.

Comment 3

[Justin Crawford [:hoosteeno] [:jcrawford]]

Thanks Margaret. 

I suggest we follow guidance in Comment 1, which modifies requirements to...

https://input.mozilla.org/feedback/%PRODUCT%/%VERSION%/%CHANNEL%?utm_source=feedback-prompt&url=http://foo.bar.com

1. utm_source=feedback-prompt just needs its param name changed -- currently it's "source". I'll have to make a small change to accommodate it.

2. url=http://foo.bar.com is pre-filled in the "last site visited" field. If not present, nothing goes in that field. In the current fennec form the field is readonly so people cannot type a URL there. I think it needs to be writeable since people can get to this form in other products and may wish to enter a URL manually.

3. %PRODUCT%/%VERSION%/%CHANNEL% should be "android/45.0/firefox" or "android/46.0beta/firefox_beta" I think, and I'll have to hack a bit to make this do what is currently done by params.

Comment 4

[:Margaret Leibovic]

Here are the standard options for URL slugs:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/urlformatter/nsURLFormatter.js#89

https://input.mozilla.org/feedback/%OS%/%VERSION%/%CHANNEL%/

..evaluates to...

https://input.mozilla.org/feedback/Android/46.0a1/default/

Does that suffice? I feel like %CHANNEL% isn't exactly what you expect here, but that is what we're currently using in about:feedback:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/aboutFeedback.js#141

Do you need any of that other device data we're currently including in about:feedback?

Comment 5

[Justin Crawford [:hoosteeno] [:jcrawford]]

> https://input.mozilla.org/feedback/Android/46.0a1/default/
> Does that suffice? 

I can probably work with this if you can help me understand all the variations I might have to handle. I looked for them in the code but don't see them. 

Regarding version/channel, I assume...

* includes "a" is an aurora version
* includes "beta" is a beta version
* includes digits/dots only is a release version

Is that correct?

I don't understand what "default" is, though.

> Do you need any of that other device data we're currently including in about:feedback?

I don't see any way for us to handle any additional information, so I don't think we do. willkg, do we capture more than these params?

Comment 6

[:Margaret Leibovic]

(In reply to Justin Crawford [:hoosteeno] [:jcrawford] from comment #5)
> > https://input.mozilla.org/feedback/Android/46.0a1/default/
> > Does that suffice? 
> 
> I can probably work with this if you can help me understand all the
> variations I might have to handle. I looked for them in the code but don't
> see them. 
> 
> Regarding version/channel, I assume...
> 
> * includes "a" is an aurora version

"a" is Nightly or Aurora. "a1" is Nightly and "a2" is Aurora (those never change, since they update every day and we don't cut new version numbers).

> * includes "beta" is a beta version

It's just "b". We ship multiple beta versions throughout the cycle so this will range from "b1" to "b10" (or higher, if we cut more betas).

> * includes digits/dots only is a release version

Yes, digits/dots only is a release version.

You can see the version numbers if you go to about:firefox (or the same is true of the "about" window on desktop firefox).

> I don't understand what "default" is, though.

I believe that's just happening because this is my local build, so it isn't on any update channel. Therefore the fallback is "default". I'm not sure what happens on our Beta/Release builds that come from the Play store, since they're not on an update channel from Mozilla. They may not have a relevant value here.

We're currently sending this data to input through about:feedback, so maybe willkg knows what we're currently seeing.

Comment 7

[Will Kahn-Greene [:willkg] ET needinfo? me]

For channel, I see these values:

mysql> select distinct channel, count(*)  from feedback_response where product = "Firefox for Android" group by channel;
+---------+----------+
| channel | count(*) |
+---------+----------+
|         |    40768 |
| aurora  |     1214 |
| beta    |     9576 |
| default |     2517 |
| nightly |     1498 |
| no      |        5 |
| release |   184429 |
| stable  |   384494 |
+---------+----------+
8 rows in set (1.04 sec)


Does that help?

Comment 8

[Will Kahn-Greene [:willkg] ET needinfo? me]

Actually, narrowing down the time range so we're not getting old stuff:

mysql> select distinct channel, count(*)  from feedback_response where product = "Firefox for Android" and created > "2015-01-01" group by channel;
+---------+----------+
| channel | count(*) |
+---------+----------+
|         |    26683 |
| aurora  |     1093 |
| beta    |     9012 |
| default |     2497 |
| nightly |     1364 |
| no      |        5 |
| release |   184429 |
+---------+----------+
7 rows in set (0.97 sec)

Comment 9

[Justin Crawford [:hoosteeno] [:jcrawford]]

Yes, it helps. It means we have those channel strings in the client and can put them in the URL.

So suppose the URL fennec requests follows this pattern...

https://input.mozilla.org/feedback/%OS%/%VERSION%/%CHANNEL%/?utm_source=feedback-prompt

My code currently will route...
1) anything with a 'b' in VERSION or a 'beta' in CHANNEL to the beta app in play store
2) anything else to the release app in play store

As before, we'll use an intent if we find 'utm_source=feedback-prompt' in params.

Will this work?

Comment 10

[:Margaret Leibovic]

(In reply to Justin Crawford [:hoosteeno] [:jcrawford] from comment #9)
> Yes, it helps. It means we have those channel strings in the client and can
> put them in the URL.
> 
> So suppose the URL fennec requests follows this pattern...
> 
> https://input.mozilla.org/feedback/%OS%/%VERSION%/%CHANNEL%/
> ?utm_source=feedback-prompt
> 
> My code currently will route...
> 1) anything with a 'b' in VERSION or a 'beta' in CHANNEL to the beta app in
> play store

I would pick one or the other. 

> 2) anything else to the release app in play store

You should make sure to only associate release builds with the release app. So we should ignore versions with an 'a' in VERSION, and only pay attention to 'release' in CHANNEL.

Given these numbers from willkg, I think it would make the most sense to just key off the release/beta channel parameters.

Comment 11

[Justin Crawford [:hoosteeno] [:jcrawford]]

> > My code currently will route...
> > 1) anything with a 'b' in VERSION or a 'beta' in CHANNEL to the beta app in
> > play store
> 
> I would pick one or the other. 
> 

Great. Please send "beta" in the CHANNEL and I'll handle that.

> You should make sure to only associate release builds with the release app.
> So we should ignore versions with an 'a' in VERSION, and only pay attention
> to 'release' in CHANNEL.

I chatted with :margaret in IRC about this. We decided to hide the play store link for everyone but release/beta users. This means the "thanks" page will be a bit abrupt for those users -- it'll just say, "That's great to hear!"

We're OK with this -- the population is small, they expect some unpolished bits, and we can certainly add new text after launch saying, "Thanks for being a nightly user" or whatever. We're just trying to avoid adding new text for launch.

Comment 12

[:Margaret Leibovic]

Sorry I've been so slow to wrap this up, I'm picking this back up and want to land it this week!

Currently I have a patch that formats the URL as:
https://input.mozilla.org/feedback/%OS%/%VERSION%/%CHANNEL%/

In my local build, this turns into:
https://input.mozilla.org/feedback/Android/47.0a1/default/

However, this redirects to a generic input.mozilla.org landing page, not the new Android feedback page. Is there something that needs updating on the input side to handle this URL?

I should note that in addition to using this new URL to prompt users after they launch the app a certain number of times, they can also get to this page through Settings -> Mozilla Firefox -> Give feedback.

Comment 13

[Will Kahn-Greene [:willkg] ET needinfo? me]

I'm not sure what happened... At some point in this bug discussion, someone started using %OS% instead of %PRODUCT%, but I'm not sure why.

I think the only url that will work is the one I suggested in comment #1:

https://input.mozilla.org/feedback/%PRODUCT%/%VERSION%/%CHANNEL%

%PRODUCT% has to be a valid value in the product table, otherwise you get sent to the product picker.

See if that works?

Comment 14

[:Margaret Leibovic]

(In reply to Will Kahn-Greene [:willkg] from comment #13)
> I'm not sure what happened... At some point in this bug discussion, someone
> started using %OS% instead of %PRODUCT%, but I'm not sure why.
> 
> I think the only url that will work is the one I suggested in comment #1:
> 
> https://input.mozilla.org/feedback/%PRODUCT%/%VERSION%/%CHANNEL%
> 
> %PRODUCT% has to be a valid value in the product table, otherwise you get
> sent to the product picker.
> 
> See if that works?

That works, thanks!

There must have been some confusion with SUMO, since I see in our SUMO links we use %OS% as the parameter.

I'm just going to hardcode "android" into these URLs, since it's not going to change for any variation of Fennec.

I'll do some more testing and then post a patch.

Comment 15

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1239513 - Replace about:feedback with page hosted on input.mozilla.org. r=sebastian

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

Comment 16

[:Margaret Leibovic]

I'm updating this bug to be about making the switch to this new hosted page.

During my testing, the one suspicious thing I found was that the "I love it" link doesn't take me to the play store, but I see if I change the URL to use "release" or "beta" as the channel slug, it works as expected.

Nice job!

Comment 17

[:Margaret Leibovic]

Sebastian, I'll file a follow-up bug to remove about:feedback from the tree. I want to hold off on doing that until we're confident that there aren't any problems with this new page.

Comment 18

[Justin Crawford [:hoosteeno] [:jcrawford]]

> During my testing, the one suspicious thing I found was that the "I love it"
> link doesn't take me to the play store, but I see if I change the URL to use
> "release" or "beta" as the channel slug, it works as expected.

Margaret, what was the channel slug in your tests? As we discussed in IRC (summarized in comment 11), any explicit channel slug not beta or release should not get a store link since there isn't a store link for that channel.

Comment 19

[Justin Crawford [:hoosteeno] [:jcrawford]]

Also, I just want to be sure that we're adding the utm_source and url parameters to the link:

https://input.mozilla.org/feedback/%PRODUCT%/%VERSION%/%CHANNEL%?utm_source=feedback-prompt&url=http://foo.bar.com

* utm_source forces the device experience (intent vs. http link)
* url is the "last url visited"

Comment 20

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8726280 [details]
MozReview Request: Bug 1239513 - Replace about:feedback with page hosted on input.mozilla.org. r=sebastian

https://reviewboard.mozilla.org/r/37897/#review34451

::: mobile/android/chrome/content/Feedback.js:28
(Diff revision 1)
> +      if (lastUrl) {
> +        url += "&url=" + encodeURIComponent(lastUrl);
> +      }

I assume this is used to pre-fill the form? But doesn't this mean by loading this external feedback URL we already "leaked" the last visited URL regardless of whether the user will use it / wants to use it in the form.

Comment 21

[:Margaret Leibovic]

(In reply to Justin Crawford [:hoosteeno] [:jcrawford] from comment #19)
> Also, I just want to be sure that we're adding the utm_source and url
> parameters to the link:
> 
> https://input.mozilla.org/feedback/%PRODUCT%/%VERSION%/
> %CHANNEL%?utm_source=feedback-prompt&url=http://foo.bar.com
> 
> * utm_source forces the device experience (intent vs. http link)
> * url is the "last url visited"

Yes, I included these parameters for the case where this is opened from the intent.

(In reply to Sebastian Kaspari (:sebastian) from comment #20)
> Comment on attachment 8726280 [details]
> MozReview Request: Bug 1239513 - Replace about:feedback with page hosted on
> input.mozilla.org. r=sebastian
> 
> https://reviewboard.mozilla.org/r/37897/#review34451
> 
> ::: mobile/android/chrome/content/Feedback.js:28
> (Diff revision 1)
> > +      if (lastUrl) {
> > +        url += "&url=" + encodeURIComponent(lastUrl);
> > +      }
> 
> I assume this is used to pre-fill the form? But doesn't this mean by loading
> this external feedback URL we already "leaked" the last visited URL
> regardless of whether the user will use it / wants to use it in the form.

Oh, this is a really good point... I should have thought of this.

I think we should probably remove this feature, then, and just do what desktop does. As much as it's annoying for the user to type out a URL for a website, it's much worse for us to violate their privacy.

Justin, would it be okay to remove this parameter from the URL? We would need to update the page to not include that checkbox anymore.

Comment 22

[Justin Crawford [:hoosteeno] [:jcrawford]]

> Justin, would it be okay to remove this parameter from the URL? We would need to update the page to not include that checkbox anymore.

We can remove that parameter without breaking anything. And we can remove the checkbox. They aren't tightly coupled, but we should do both before launch.

Comment 23

[Justin Crawford [:hoosteeno] [:jcrawford]]

> I should note that in addition to using this new URL to prompt users after they launch the app a certain number of times, they can also get to this page through Settings -> Mozilla Firefox -> Give feedback.

> Yes, I included these parameters for the case where this is opened from the intent.


This points out a use case we haven't coded for. Right now, the parameter is how we determine that someone is on a device, and that's how we decide whether to use an intent or a URL for the play store. I think we need a second parameter for "On device but not coming from feedback prompt". I suggest when people choose "give feedback," we send a param, "utm-source=feedback_link". We'll need to handle it in the page; again, not tightly coupled, but we'll need to do both before launch. Sound good?

Comment 24

[Justin Crawford [:hoosteeno] [:jcrawford]]

Arg. The param above should be, "utm_source=feedback-link".

Comment 25

[:Margaret Leibovic]

(In reply to Justin Crawford [:hoosteeno] [:jcrawford] from comment #23)
> > I should note that in addition to using this new URL to prompt users after they launch the app a certain number of times, they can also get to this page through Settings -> Mozilla Firefox -> Give feedback.
> 
> > Yes, I included these parameters for the case where this is opened from the intent.
> 
> 
> This points out a use case we haven't coded for. Right now, the parameter is
> how we determine that someone is on a device, and that's how we decide
> whether to use an intent or a URL for the play store. I think we need a
> second parameter for "On device but not coming from feedback prompt". I
> suggest when people choose "give feedback," we send a param,
> "utm-source=feedback_link". We'll need to handle it in the page; again, not
> tightly coupled, but we'll need to do both before launch. Sound good?

(In reply to Justin Crawford [:hoosteeno] [:jcrawford] from comment #24)
> Arg. The param above should be, "utm_source=feedback-link".

Sure, I can do that. But maybe instead of feedback-link it should be feedback-settings? This doesn't appear as a link in settings, but rather as a settings item.

I'm fine either way, just want to make sure that people looking at this later aren't confused :)

Comment 26

[Justin Crawford [:hoosteeno] [:jcrawford]]

feedback-settings works for me.

Comment 27

[:Margaret Leibovic]

Comment on attachment 8726280 [details]
MozReview Request: Bug 1239513 - Replace about:feedback with page hosted on input.mozilla.org. r=sebastian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37897/diff/1-2/

Comment 28

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/62a334ac99eb9de2e0199db428e0182ea1ceb98a
Bug 1239513 - Replace about:feedback with page hosted on input.mozilla.org. r=sebastian

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/62a334ac99eb

Comment 30

[Teodora Vermesan (:TeoVermesan)]

Nightly 48.0a1 (2016-03-13)
Open Nightly, then tap the home button and repeat this 14 times, the 15th time will open a new tab with: 
https://input.mozilla.org/en-US/feedback/android/48.0a1/nightly/?utm_source=feedback-prompt

1. Going to about:feedback will display "File not Found. Firefox can't find the file at about:feedback". Is it expected? 
2. Choosing "I ran into some problems" and tapping back, "Visit out Support Site to get answers for common issues" is still displayed and cannot be dismissed. I've filled Bug 1256247.

Aurora 47.0a2 (2016-03-09)
Open Aurora, then tap the home button and repeat this 14 times, the 15th time will open a new tab with: 
https://input.mozilla.org/en-US/feedback/android/47.0a2/aurora/?utm_source=feedback-prompt

1. Going to about:feedback will display the page. Is it expected?
2. On input.mozilla.org page, choosing "I ran into some problems" and tapping back, "Visit out Support Site to get answers for common issues" is still displayed and cannot be dismissed.
3. On about:feedback, works ok.

Comment 31

[:Margaret Leibovic]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #30)
> Nightly 48.0a1 (2016-03-13)
> Open Nightly, then tap the home button and repeat this 14 times, the 15th
> time will open a new tab with: 
> https://input.mozilla.org/en-US/feedback/android/48.0a1/nightly/
> ?utm_source=feedback-prompt
> 
> 1. Going to about:feedback will display "File not Found. Firefox can't find
> the file at about:feedback". Is it expected? 

Yes, that's expected.

> 2. Choosing "I ran into some problems" and tapping back, "Visit out Support
> Site to get answers for common issues" is still displayed and cannot be
> dismissed. I've filled Bug 1256247.

Thanks for filing.

> Aurora 47.0a2 (2016-03-09)
> Open Aurora, then tap the home button and repeat this 14 times, the 15th
> time will open a new tab with: 
> https://input.mozilla.org/en-US/feedback/android/47.0a2/aurora/
> ?utm_source=feedback-prompt
> 
> 1. Going to about:feedback will display the page. Is it expected?

Yes, I didn't uplift bug 1253339. I wanted to leave the door open for us in case we want to revert back to about:feedback, but I'm starting to think we should just uplift that change as well.

> 2. On input.mozilla.org page, choosing "I ran into some problems" and
> tapping back, "Visit out Support Site to get answers for common issues" is
> still displayed and cannot be dismissed.

Yeah, same as bug 1256247.

> 3. On about:feedback, works ok.

Good for comparison, but we're going to remove about:feedback, so this won't matter.

Comment 32

[Teodora Vermesan (:TeoVermesan)]

Open 47 Beta 1, then tap the home button and repeat this 14 times, the 15th time will open a new tab with: 
https://input.mozilla.org/en-US/feedback/android/47.0/beta/?utm_source=feedback-prompt

Verified as fixed using One A2001 (Android 5.1.1)