Closed Bug 955079 Opened 10 years ago Closed 10 years ago

The value of list type options is not always preserved

Categories

(Instantbird Graveyard :: Account wizard, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wnayes, Assigned: wnayes)

References

Details

Attachments

(1 file, 3 obsolete files)

*** Original post on bio 1650 at 2012-08-15 18:35:00 UTC ***

When setting advanced options for an account, changes to the value of list-type options are not always preserved. Specifically, they are not kept if a non-default value is selected, the page is advanced and the user does not return to the advanced options page again.

To reproduce:
  1. Enter the wizard and select a protocol with a list type option (for example, AIM)
  2. Enter any username or password.
  3. On the page with the alias selection, open the AIM Options groupbox and change the encryption dropdown to a non-default.
  4. After hitting next, the option is not listed in the summary and creating the account will not preserve it.

If you instead go Back from the summary page, select the non-default value again, and hit Next, the value will be shown as selected. There is something strange about accessing the value property on menulist items, causing the old value to be returned.
*** Original post on bio 1650 as attmnt 1807 at 2012-08-15 18:38:00 UTC ***

This fixes the bug by reading the value attribute (instead of property) on menulist items. I am not sure how it will apply, as I can see my import wizard changes in the surrounding lines.
Attachment #8353566 - Flags: review?(clokep)
Assignee: nobody → wnayes
Status: NEW → ASSIGNED
Comment on attachment 8353566 [details] [diff] [review]
Read the value attribute on menulist tags

*** Original change on bio 1650 attmnt 1807 at 2012-08-15 18:56:16 UTC ***

As discussed on IRC, the value of a menulist isn't actually the value of the selected element: https://developer.mozilla.org/en-US/docs/XUL/menulist#a-value

I suggest using if ("selectedItem" in elt) return elt.selectedItem.value; since we wouldn't need any further explanation then.
Attachment #8353566 - Flags: review?(clokep) → review-
*** Original post on bio 1650 as attmnt 1808 at 2012-08-15 18:57:00 UTC ***

Reads the list option value from the selectedItem if available. Drops back to the value property when the options have never been viewed (and no selectedItem exists).
Attachment #8353567 - Flags: review?(clokep)
Comment on attachment 8353566 [details] [diff] [review]
Read the value attribute on menulist tags

*** Original change on bio 1650 attmnt 1807 at 2012-08-15 18:57:15 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353566 - Attachment is obsolete: true
*** Original post on bio 1650 as attmnt 1809 at 2012-08-15 19:04:00 UTC ***

Did not see the review comment until posting my previous patch. The check for selectedItem is a cleaner way to fix this.
Attachment #8353568 - Flags: review?(clokep)
Comment on attachment 8353567 [details] [diff] [review]
Read the list type option value from the selectedItem

*** Original change on bio 1650 attmnt 1808 at 2012-08-15 19:04:36 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353567 - Attachment is obsolete: true
Attachment #8353567 - Flags: review?(clokep)
Blocks: 954928
Attached patch Patch for trunkSplinter Review
*** Original post on bio 1650 as attmnt 1860 at 2012-08-28 01:25:00 UTC ***

Patch that applies to trunk.
Comment on attachment 8353568 [details] [diff] [review]
Alternate way of checking for selectedItem

*** Original change on bio 1650 attmnt 1809 at 2012-08-28 01:25:02 UTC ***

This looks good, I'm going to upload a patch that applies to the current trunk.
Attachment #8353568 - Flags: review?(clokep) → review+
Comment on attachment 8353568 [details] [diff] [review]
Alternate way of checking for selectedItem

*** Original change on bio 1650 attmnt 1809 at 2012-08-28 01:26:16 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353568 - Attachment is obsolete: true
Whiteboard: [checkin-needed]
*** Original post on bio 1650 at 2012-08-30 02:48:26 UTC ***

Committed as http://hg.instantbird.org/instantbird/rev/e11462508947

Thanks Will!
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.3
*** Original post on bio 1650 at 2012-09-26 00:50:22 UTC ***

Marking this as resolved as it has been committed and no longer blocks Bug 954928 (bio 1495) :).
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.