623679 - Add unique classes to fields prefilled by desktop and mobile feedback add-ons.

Status: RESOLVED FIXED

(Input :: General, defect, P1)

Description

[Fred Wenzel [:wenzel]]

We used to prefill fields by ID, but now that we have several feedback forms in the same DOM tree we need to change to classes instead of IDs.

For Desktop and Mobile:
- Do not look for #id_url anymore, but for: .url

For Mobile:
- Do not look for #manufacturer and #device anymore but for:
-- .manufacturer
-- .device


I'll change the Input side of things as part of this bug.

Comment 1

[Fred Wenzel [:wenzel]]

When you look for these classes, just loop over all input fields you find, and prefill them as we did with the single field before.

CCing Jono and mfinkle.

Comment 2

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

Attachment: patch for fennec

This patch adds support for filling hidden <input> based on class name. I kept the code to fill by ID for now.

Fred - please check the class names I am using in the patch. Also, where can I test this?

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 501910 [details] [diff] [review]
patch for fennec

Adding a "let json = aMessage.json;" and using it at the beginning of this function won't hurt.
Unrelated but I've noticed the content.js code is loaded outside of the UIReady event listener, do you know if there is any reason for that?

Comment 4

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

pushed mobile patch (with Vivien's suggestions)
http://hg.mozilla.org/mobile-browser/rev/a7bffe8d7e5d

Not closing bug since desktop needs to be fixed too.

Comment 5

[Fred Wenzel [:wenzel]]

Sorry I am a little late to the game, but the part where you prefill based on the ID can be removed now, I think. I just added the classes:

http://github.com/fwenzel/reporter/commit/5f57c96

... and you can test it working on input.stage.mozilla.com when it it picked up in a few minutes.

I'll keep the IDs around for a little while, but once it is all deployed in the client, they are going away in favor of the classes.

Comment 6

[Fred Wenzel [:wenzel]]

Calling this one closed: Jono is working on the changes to the Desktop add-on, but the Mobile add-on is more important (as we don't get the manufacturer and device data any other way). Thanks everyone!

Comment 7

[Jono Xia]

I've implemented the change for the desktop version over in Bug 617393 but the patch is waiting for some icons and some decisions on strings before it can be merged.

Comment 8

[Rebecca Billings [:rbillings]]

Manually setting the about:config to redirect feedback to stage, no urls are populated

Comment 9

[Fred Wenzel [:wenzel]]

Aakashd, I cannot get the desktop add-on to prepopulate the URL anymore by clicking the "makes me happy"/"makes me sad" button. Can you confirm? If this is nothign we can fix on our side, then we should re-close this bug (and not block tomorrow's push), but nonetheless get the desktop add-on fixed.

Comment 10

[Aakash Desai [:aakashd]]

Yep, that's happening to me too. 

The changes landed on testpilot:  http://hg.mozilla.org/labs/testpilot/rev/7daa876984df but it hasn't landed onto Firefox's feedback add-on it seems.

Comment 11

[Fred Wenzel [:wenzel]]

Note that the node ID #id_url is still present for now, so the old version of the feedback add-on should still prefill the URL field just fine. :-/

Comment 12

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

I found the bug. What is the URL for the stage? I need to test.

Comment 13

[Fred Wenzel [:wenzel]]

(In reply to comment #12)
> I found the bug. What is the URL for the stage? I need to test.

http://input.stage.mozilla.com/en-US/sad/

(note that post 3.0, this will change to ..../beta/sad).

Just wondering, it's just the mobile add-on you're looking at, not the desktop one, right?

Comment 14

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

(In reply to comment #13)
> (In reply to comment #12)

> Just wondering, it's just the mobile add-on you're looking at, not the desktop
> one, right?

correct

Comment 15

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

Hmm, I am using a developer Android build and when I go to http://input.stage.mozilla.com/en-US/sad/ I am prompted to download Firefox Mobile.

How can I test this fix?

Comment 16

[Fred Wenzel [:wenzel]]

Can you change your user agent string? If so, pretend to be the latest Firefox mobile beta. (Two things are important: Pretend to use a beta, not nightly, and make sure you use a version that's equal or newer to the latest known beta.)

Comment 17

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

Attachment: patch 2

getElementsByClassName does not return an array. This patch switches to for(;;) syntax.

Comment 18

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

Grrrr - I'll remove the Makefile.in changes before checkin

Comment 19

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

pushed Fennec changes needed for beta:
http://hg.mozilla.org/mobile-browser/rev/99268b046596