Closed Bug 1071117 Opened 10 years ago Closed 10 years ago

Add support to distinguish organic and inorganic feedback.

Categories

(Firefox for Android Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox35 fixed, firefox36 fixed)

RESOLVED FIXED
Firefox 36
Tracking Status
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: vivek, Assigned: vivek, Mentored)

References

Details

Attachments

(1 file, 2 obsolete files)

This is follow up bug for #1066062. Currently source field is not set while submitting the negative feedback. W.r.t https://bugzilla.mozilla.org/show_bug.cgi?id=1066062#c3 , this bug tracks source field when user is prompted
Assignee: nobody → vivekb.balakrishnan
Mentor: margaret.leibovic
Depends on: 799562
Attached patch source_organic.patch (obsolete) — Splinter Review
Patch with fix :
Source field is set only when fennec prompts the user for feedback.
Attachment #8503499 - Flags: review?(margaret.leibovic)
Comment on attachment 8503499 [details] [diff] [review]
source_organic.patch

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

f+ for now, because I think we should use a source name that's less ambiguous than "about:feedback".

::: mobile/android/chrome/content/aboutFeedback.js
@@ +149,5 @@
> +  let getParam = window.location.href.split("?");
> +  if (getParam.length > 1) {
> +    let urlParam = new URLSearchParams(getParam[1]);
> +    if(urlParam.get("non_organic") == 1) {
> +      data["source"] = "about:feedback";

Let's check with willkg about what kinds of values are normally passed here. Since organic feedback can come from about:feedback as well, it might be better to set the source to "launch-new-tab" (or something like tha), to indicate that this is coming from the new tab we open when the user launches the browser.

Also, maybe one day we'll have more than one way to ask the user for feedback, such as from a snippet.

We could also change the parameter on the URL from "non_organic" directly to "source", and then if there's a source parameter we can just pass that directly along to input.
Attachment #8503499 - Flags: review?(margaret.leibovic) → feedback+
We just talked about this on the I R C:

<vivek> willkg, I have question related to the source field. This is related to #1071117
* willkg nods.
<vivek> Are there any restrictions for them. Also, is about:feedback a good source field value?
<willkg> there are no restrictions except for length.
<willkg> the value should be whatever initiated the feedback.
<willkg> i'm not sure about:feedback is a good value. how does that initiate the ask for feedback?
<vivek> Ok, the feedback is actually initiated after the user has launched fennec certain amount of times. May be about:feedback is not the right value as you say.
<willkg> Cww: ^^^ you around to bounce better names off of?
<willkg> Cww: if fennec prompts the user for feedback after the user has launched fennec x times, what would a helpful "source" value be?
<Cww> um, source = prompt is probably fine
<Cww> is this also the google play diversion piece?
<vivek> This for the negative feedback.
<Cww> vivek: sorry, this is what I meant, it's the other half of the google play diversion piece
<vivek> Cww,  yes
<Cww> I think feedback-prompt is probably good.
<vivek> Cww willkg ,  thanks.
<Cww> as long as we know what it is, we're all set.
<willkg> bd
<Cww> thanks for fixing the bug :)
<vivek> willkg, can you please comment on the bug?
<willkg> sure!
<vivek> willkg, thanks
Attached patch source_organic.patch (obsolete) — Splinter Review
A new patch with source value set to feedback-prompt
Attachment #8503499 - Attachment is obsolete: true
Attachment #8504979 - Flags: review?(margaret.leibovic)
Comment on attachment 8504979 [details] [diff] [review]
source_organic.patch

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

Looking good!

::: mobile/android/chrome/content/aboutFeedback.js
@@ +144,5 @@
>    data["version"] = Services.appinfo.version;
>    data["locale"] = Services.locale.getSystemLocale().getCategory("NSILOCALE_CTYPE");
>    data["channel"] = UpdateChannel.get();
>  
> +  // Bug 1071117: source field is added only when Fennec prompts user.

Nit: You don't need to include the bug number in the comment, since it will appear in hg blame.
Attachment #8504979 - Flags: review?(margaret.leibovic) → review+
Minor nit corrected
Attachment #8504979 - Attachment is obsolete: true
Keywords: checkin-needed
Comment on attachment 8504997 [details] [diff] [review]
source_organic.patch

Approval Request Comment
[Feature/regressing bug #]: follow-up to bug 799562
[User impact if declined]: we will have less information about where feedback is coming from
[Describe test coverage new/current, TBPL]: no automated tests, tested manually
[Risks and why]: low-risk, adds parameter to feedback submission
[String/UUID change made/needed]: none
Attachment #8504997 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/d369649bb265
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
Attachment #8504997 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.