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

Status: RESOLVED FIXED

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

Description

[Ian Neal]

Attachment: Port extendability

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

Comment 1

[Philip Chee]

>    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?

Comment 2

[Ian Neal]

Attachment: Port extendability with for let of

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).

Comment 3

[Ian Neal]

Attachment: With added portness [Checked in: Comment 5]

Changes since last version:
* Added port of Bug 514666 - Rework EditCardOKButton: use .equals() to compare nsIAbCards instead of .indexOf()

Comment 4

[Karsten Düsterloh]

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.

Comment 5

[Ian Neal]

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