451381 - Re-use the new address book popup widget on Thunderbird's preference pane

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect, P3)

Description

[Mark Banner (:standard8)]

Attachment: The fix

Now that we have a new address book popup widget for listing the address books, we should use it on Thunderbird's preference window.

This means we can drop lots of unneeded js code and benefit from some of the fixes it provides (updates when AB names change, simplification etc).

The patch I'm attaching depends on the partially reviewed patch in bug 450581.

Comment 1

[Magnus Melin [:mkmelin]]

Comment on attachment 334687 [details] [diff] [review]
The fix

Seems there's a small problem with this patch: the "Directory Server:" dropdown doesn't have a "None" like it used to, and the dropdown is just a few pixels high. Clicking around on the tiny height dropdown it gives a js error in the console 

Error: enableElement is not defined
Source File: chrome://messenger/content/preferences/compose.js
Line: 68

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 334687 [details] [diff] [review]
The fix

Magnus, did you apply the patch from bug 450581 first? (attachment 334259 [details] [diff] [review])

Just a note, I've also dropped the none as the new popups should show the state a lot better in that case (i.e. not pretend they are set to a different directory which was the bug when we introduced the none).

Comment 3

[Magnus Melin [:mkmelin]]

Mark: yes, I applied that patch first.
I don't mind about the "None" missing (and some other bug also mentioned that), but should I be seeing the just a few pixel high dropdown then? At least clicking on it shouldn't cause js errors. 

Comment 4

[Mark Banner (:standard8)]

Attachment: The fix v2

Revised patch. When I was implementing the SM equivalent, Neil reminded me that the default for the preference was "", so having a "None" option highlighted seems preferable - therefore I've put it back the way it was.

Also fixed enable/disable of the LDAP menu & edit button, added in the locked preference option that SM has, and I don't understand why it isn't in TB.

Also updated a few things for the changes that have gone into addrbookWidgets.xml in bug 450581.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 337641 [details] [diff] [review]
The fix v2

Looks good, r=mkmelin

Comment 6

[Mark Banner (:standard8)]

Checked in, changeset id: 301:94bb5b89003e