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

RESOLVED FIXED in Thunderbird 3.0a3

Status

defect
P3
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 3.0a3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

11 years ago
Posted patch The fix (obsolete) — Splinter Review
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.
Attachment #334687 - Flags: review?(mkmelin+mozilla)
Assignee

Updated

11 years ago
Priority: -- → P3
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
Attachment #334687 - Flags: review?(mkmelin+mozilla) → review-
Assignee

Comment 2

11 years ago
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).
Attachment #334687 - Flags: review- → review?(mkmelin+mozilla)
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. 
Assignee

Comment 4

11 years ago
Posted patch The fix v2Splinter Review
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.
Attachment #334687 - Attachment is obsolete: true
Attachment #337641 - Flags: review?(mkmelin+mozilla)
Attachment #334687 - Flags: review?(mkmelin+mozilla)
Comment on attachment 337641 [details] [diff] [review]
The fix v2

Looks good, r=mkmelin
Attachment #337641 - Flags: review?(mkmelin+mozilla) → review+
Assignee

Comment 6

11 years ago
Checked in, changeset id: 301:94bb5b89003e
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b1
You need to log in before you can comment on or make changes to this bug.