Closed Bug 610494 Opened 9 years ago Closed 9 years ago

allow for device type to be sent within mobile submission pages

Categories

(Input :: General, defect, P1, major)

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: aakashd, Assigned: wenzel)

References

Details

Attachments

(2 files)

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?
Depends on: 604712
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.
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.
Attached patch fennec patchSplinter Review
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.
Attachment #491276 - Flags: review?(mbrubeck)
Comment on attachment 491276 [details] [diff] [review]
fennec patch

>+++ b/app/profile/extensions/feedback@mobile.mozilla.org/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/feedback@mobile.mozilla.org/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.
Attachment #491276 - Flags: review?(mbrubeck) → review+
(In reply to comment #4)
> Comment on attachment 491276 [details] [diff] [review]
> fennec patch
> 
> >+++ b/app/profile/extensions/feedback@mobile.mozilla.org/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 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
Assignee: nobody → fwenzel
Severity: enhancement → major
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.
Status: NEW → ASSIGNED
I believe this is fixed now, no?
Oh yes, it is. Thanks!
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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.
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.
Status: RESOLVED → VERIFIED
Attached image Post-fix screenshot
(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!
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?
(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.
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.
Component: Input → General
Product: Webtools → Input
You need to log in before you can comment on or make changes to this bug.