Closed Bug 761852 Opened 9 years ago Closed 9 years ago

Port |Bug 664726 - Add hooks to make address book more extend-able| and follow-ups to SeaMonkey

Categories

(SeaMonkey :: MailNews: Address Book & Contacts, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.13

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch Port extendability (obsolete) — Splinter Review
To provide better extension compatibility, it would be good to port:
* Bug 664726 - Add hooks to make address book more extend-able
* Bug 690655 - Fix regression caused by bug 664726 that prevents users from editing their identity vCards
* Bug 702137 - genericPhotoHandler saves wrong photoname
Attachment #630362 - Flags: review?(mnyromyr)
>    var i;
>    // create a list of mailing lists and the index where the card is at.
> -  for ( i=0;  i < listDirectoriesCount; i++ ) {
> +  for (i = 0; i < listDirectoriesCount; i++) {

> -  for (i=0; i<foundDirectoriesCount; i++) {
> +  for (i = 0; i < foundDirectoriesCount; i++) {

We can get rid of var i if we use:
for (let i = 0; i < listDirectoriesCount; i++) {
etc.

> +  for (var i = 0; i < gOnLoadListeners.length; i++)
> +    gOnLoadListeners[i](aCard, aDoc);
for (let i =

I think you can possibly do:
for (let listener of gOnLoadListeners)
  listener(aCard, aDoc)

I wonder if it is feasible to abuse nsIFaviconService to store these pics?
Changes since last version:
* Used let in for as suggested.
* Used for (let listener of...) as suggested.

Abusing nsIFaviconService would be out of scope of this bug I think (not sure how workable it is either).
Attachment #630362 - Attachment is obsolete: true
Attachment #630362 - Flags: review?(mnyromyr)
Attachment #630772 - Flags: review?(mnyromyr)
Changes since last version:
* Added port of Bug 514666 - Rework EditCardOKButton: use .equals() to compare nsIAbCards instead of .indexOf()
Attachment #630772 - Attachment is obsolete: true
Attachment #630772 - Flags: review?(mnyromyr)
Attachment #630948 - Flags: review?(mnyromyr)
Depends on: 514666
Comment on attachment 630948 [details] [diff] [review]
With added portness [Checked in: Comment 5]

> var gOnSaveListeners = new Array();
>+var gOnLoadListeners = new Array();

I'd prefer initing to [] (like you do in other places).

>+  for (let i = 0; i < listDirectoriesCount; i++) {

Please put braces on their own line, here and "everywhere" in the rest of the patch. 
(The files' style is mostly mixed in this regard, let's at least fix those you touch.)

>+    foundItem.directory
>+             .addressLists
>+             .replaceElementAt(gEditCard.card, foundItem.cardIndex, false);

Hm. Probably better to just not wrap it.

>+function RegisterLoadListener(aFunc)
>+{
>+  gOnLoadListeners[gOnLoadListeners.length] = aFunc;

Using .push (like you did above in OnLoadNewCard) would be way cleaner.

>+/* Registers functions that are called when saving the card
>+ * values.  This is useful if extensions have added extra
>+ * fields to the user interface, and need to set those values
>+ * in their nsIAbCard.
>+ */
> function RegisterSaveListener(func)
> {
>   gOnSaveListeners[gOnSaveListeners.length] = func;

Please get rid of the ugliness here as well, while you're at it.

>+var genericPhotoHandler = {
>+>+}

The { should go on the next line and the final ; is missing.
Same goes for filePhotoHandler and webPhotoHandler — and actually, these handler objects are global variables and hence should have the 'g' prefix. I don't expect extensions to mess with these handlers directly, but add their own instead, so this should be safe.

>           <html:img id="photo" style="max-width: 25ch; max-height: 25ch;"/>

(BTW: I noticed that the photo ratio is kept in the card edit dialog, but forced to be square in the main addressbook view — is this a known bug?)

>+var genericPhotoDisplayHandler = function(aCard, aImg)
>+var photoNameDisplayHandler = function(aCard, aImg)

Same problems as above: Missing 'g' prefix, missing final ';'.


r/moa=me with these fixed.
Attachment #630948 - Flags: superreview+
Attachment #630948 - Flags: review?(mnyromyr)
Attachment #630948 - Flags: review+
Comment on attachment 630948 [details] [diff] [review]
With added portness [Checked in: Comment 5]

Checked in with requested fixes:
http://hg.mozilla.org/comm-central/rev/175f81ed0caa
Attachment #630948 - Attachment description: With added portness → With added portness [Checked in: Comment 5]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.13
You need to log in before you can comment on or make changes to this bug.