610494 - allow for device type to be sent within mobile submission pages

Status: VERIFIED FIXED

(Input :: General, defect, P1)

Description

[Aakash Desai [:aakashd]]

The mobile team is requesting device type information since the feedback we're receiving is less useful without that modifier. They're going to allow us to hook to the same device type information that's passed along to socorro, so it'll need to be posted with the rest of our form.

Things to Determine:
* Paste into the user agent received and parse through it or create a new category?
* Dashboard modifiers just like Platform and Locale
* Security thoughts?

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

We can add data from bug 604712. The feedback add-on will look for 2 fields and if those fields are found, we'll populate the data.

The fields are for: manufacturer and device

I just need the id's of the fields. You don't even need to wait until the page changes are live.

Comment 2

[Fred Wenzel [:wenzel]]

Let's go with #id_manufacturer and #id_device .

^^^ Implementation info: These will need to be the field IDs in the HTML document and will be hardcoded on the Fennec side, thus must not change.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: fennec patch

This is the patch for Fennec. It uses the above field names and depends on bug 604712.

The patch turns feedback on for Fennec nightlies again so we can test this before beta 3. We will revert the nightly part of the patch when this is verified to work.

Comment 4

[Matt Brubeck (:mbrubeck)]

Comment on attachment 491276 [details] [diff] [review]
fennec patch

>+++ b/app/profile/extensions/[e-mail]/content/content.js

>+  let device = content.document.getElementById("id_device");
>+  if (device)
>+    device.value = aMessage.json.device;
>+
>+  let manufacturer = content.document.getElementById("id_manufacturer");
>+  if (manufacturer)
>+    manufacturer.value = aMessage.json.manufacturer;

If json.device or json.manufacturer is null, then I think this will insert "null" into the form field.  You might want to do:

  device.value = aMessage.json.device || "";

>+++ b/app/profile/extensions/[e-mail]/content/overlay.js

>+      let sysInfo = Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2);
>+      Feedback._device = sysInfo.get("device");
>+      Feedback._manufacturer = sysInfo.get("manufacturer");

I assume a mozilla-central patch is coming to add "manufacturer" to nsSystemInfo.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #4)
> Comment on attachment 491276 [details] [diff] [review]
> fennec patch
> 
> >+++ b/app/profile/extensions/[e-mail]/content/content.js
> 
> >+  let device = content.document.getElementById("id_device");
> >+  if (device)
> >+    device.value = aMessage.json.device;
> 
> If json.device or json.manufacturer is null, then I think this will insert
> "null" into the form field.  You might want to do:
> 
>   device.value = aMessage.json.device || "";

Done

> >+      let sysInfo = Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2);
> >+      Feedback._device = sysInfo.get("device");
> >+      Feedback._manufacturer = sysInfo.get("manufacturer");
> 
> I assume a mozilla-central patch is coming to add "manufacturer" to
> nsSystemInfo.

bug 604712

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 491276 [details] [diff] [review]
fennec patch

pushed:
http://hg.mozilla.org/mobile-browser/rev/bdfc0633f975

When the dependent bugs also land, this will just start to work

Comment 7

[Fred Wenzel [:wenzel]]

https://github.com/fwenzel/reporter/compare/mobile-deviceinfo-610494

Dave Dash: r?

Note that this will not land until we've cleared the tree for 2.1. And it'll need to be re-merged after bug 571273 lands.

Comment 8

[Aakash Desai [:aakashd]]

I believe this is fixed now, no?

Comment 9

[Fred Wenzel [:wenzel]]

Oh yes, it is. Thanks!

Comment 10

[Fred Wenzel [:wenzel]]

QA: if you add some feedback from Fennec, their feedback extension should fill in Manufacturer and Device automatically, which we can see in the database, or by looking at the search filters a few minutes later.

I'll try and submit some feedback from my cell phone to help you verify.

Comment 11

[Fred Wenzel [:wenzel]]

I just verified this:
- I downloaded the Fennec nightly from December 15 (today's nightly does not show feedback buttons, a known bug).
- Then I pointed the happy feedback config option (extensions.feedback.url.happy) to: http://m.input.stage.mozilla.com/happy .
- I left feedback by clicking the feedback button.
- I checked in our admin interface what was submitted, and yes the manufacturer and device were properly submitted.

Screenshot to follow.

Comment 12

[Fred Wenzel [:wenzel]]

Attachment: Post-fix screenshot

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #11)

> - I downloaded the Fennec nightly from December 15 (today's nightly does not
> show feedback buttons, a known bug).

the Feedback add-on is not supposed to be in nightlies anymore. Bug 596037.

> Screenshot to follow.

Nexus S ?! Nice!

Comment 14

[Fred Wenzel [:wenzel]]

Thanks for the clarification -- that explains why the button does not show up ;) Can I install the feedback add-on independently if I want to use it on a nightly?

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #14)
> Thanks for the clarification -- that explains why the button does not show up
> ;) Can I install the feedback add-on independently if I want to use it on a
> nightly?

There's no way to do that yet. Maybe we could add it to the NTT for Mobile add-on.

Comment 16

[Aakash Desai [:aakashd]]

I'd rather not put it into NTT, but I can just copy & paste into a new add-on that replicates the feedback submission types for testing purposes.