Closed Bug 623679 Opened 9 years ago Closed 9 years ago

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

Categories

(Input :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wenzel, Assigned: wenzel)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Assignee: nobody → fwenzel
Blocks: 615645
Blocks: 617997
No longer blocks: 615645, 617997
Priority: -- → P1
Blocks: 615645, 617997
Attached patch patch for fennec (obsolete) — Splinter Review
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 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?
Attachment #501910 - Flags: review?(21) → review+
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.
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.
Status: NEW → ASSIGNED
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!
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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.
Manually setting the about:config to redirect feedback to stage, no urls are populated
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
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.
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. :-/
I found the bug. What is the URL for the stage? I need to test.
(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?
(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
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?
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.)
Attached patch patch 2Splinter Review
getElementsByClassName does not return an array. This patch switches to for(;;) syntax.
Attachment #501910 - Attachment is obsolete: true
Attachment #506967 - Flags: review?(mbrubeck)
Grrrr - I'll remove the Makefile.in changes before checkin
Attachment #506967 - Flags: review?(mbrubeck) → review+
pushed Fennec changes needed for beta:
http://hg.mozilla.org/mobile-browser/rev/99268b046596
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Component: Input → General
Product: Webtools → Input
You need to log in before you can comment on or make changes to this bug.