Enable Mac OS X system address book per default and add UI

NEW
Unassigned

Status

defect
11 years ago
8 years ago

People

(Reporter: standard8, Unassigned)

Tracking

(Blocks 1 bug)

Dependency tree / graph
Bug Flags:
wanted-seamonkey2.0 -

SeaMonkey Tracking Flags

(seamonkey2.2-, seamonkey2.3-, seamonkey2.4-)

Details

(Whiteboard: [l10n-impact])

Attachments

(1 attachment, 3 obsolete attachments)

Posted patch Possible fix (obsolete) — Splinter Review
Like Thunderbird did in bug 397811, enable the Mac OS X address book link by default in SeaMonkey and add UI to enable it manually if the user deletes it at some stage.

I've not heard any negative comments about the Mac OS X address book interface, so I don't see a problem for SeaMonkey.

Patch attached that I think works (I did it a week or so ago, but I'm not sure how much I tested it).
Posted patch The fix (obsolete) — Splinter Review
This patch ports the required changes and works (the previous one didn't quite).

Having not heard any negative comments about the Mac OS X address book on Thunderbird, I don't think there's any problem with enabling it by default on mac.
Attachment #323293 - Attachment is obsolete: true
Attachment #324138 - Flags: superreview?(neil)
Attachment #324138 - Flags: review?(mnyromyr)
I've written some alternative UI that handles the Windows cases, but it's a pity they're busted right now - I wonder why that could be :-P

IMHO enabling the Mac OS X address book by default is a separate issue.
Neil, not sure what you are talking about here regarding to Windows, but from how I read it, this bug right here is about enabling the Mac OS X address book by default, and I think it's a good idea to do that, whatever other separate issues might be (I hope there are other bugs for those).
(In reply to comment #2)
> IMHO enabling the Mac OS X address book by default is a separate issue.

I don't mind for alternatives for the UI set up. I just ported what I did on Thunderbird.

If you have issues about enabling Mac OS X address book by default, please could you raise them somewhere (m.d.a.seamonkey?) so that we can actually have a discussion about them, rather than not making a decision because we don't know what the reasons are.
Posted patch Alternative approach (obsolete) — Splinter Review
* Allows the user to add Mac OS X or Windows or MAPI address books.
* Prevents renaming of Windows/MAPI address books.

I was considering prefilling rather than hiding the address book type radiogroup when reading the properties of an existing address book.
Attachment #324802 - Flags: review?(mnyromyr)
(In reply to comment #5)
> Created an attachment (id=324802) [details]
> Alternative approach
> 
> * Allows the user to add Mac OS X or Windows or MAPI address books.
> * Prevents renaming of Windows/MAPI address books.

Whilst I like the style of the dialog, what I don't like is the fact that the way you access it (File -> New...) implies you are going to create a new XXX address book, rather than enabling a link to the existing address book (see bug 397811 comments 8, 9, 10, 11).

Maybe its just how I think about the UI. How's this for an alternative:

- Keep the File -> New -> Address Book process.
- Possibly drop the "Address Book Type:" label
- Change the label for outlook/oe/OS X address books, prefixing it with "Link to ", i.e. "Link to Mac OS X Address Book".

That way we could still combine the items into one menu option/dialog, but actually have correct wording as to what we're really going to do. 

Thoughts?

btw its "Mac OS X" not "Mac OSX"

> I was considering prefilling rather than hiding the address book type
> radiogroup when reading the properties of an existing address book.

IMHO that would be confusing, as it would imply that under some circumstances you may be able to change the type of the address book (but, of course, that's never allowed).

If I read the patch correct, then on Linux (which has no other AB types apart from mork & LDAP), the dialog will always show "Personal Address Book", I'm not sure it really needs to do that.

Is this patch set up not to display Outlook/OE address books if those apps aren't install/set as default? I thought you were going to do that and I think you should.

Also as it currently stands this patch will break Thunderbird as we share the dialog. If Bryan is happy with the possible changes then I'm happy to include the Mac part in Thunderbird at least.

I'm not convinced the Outlook/OE code is really up to production standard with there being a requirement for it to be set up as the default application.
(In reply to comment #6)
> Whilst I like the style of the dialog, what I don't like is the fact that the
> way you access it (File -> New...) implies you are going to create a new XXX
> address book, rather than enabling a link to the existing address book

If we wouldn't have "New" as a standard in all components, I'd like "Add" better there ;-)
Do we have a context menu on the tree that lists abooks that allows acces to adding such an address book as well?

> btw its "Mac OS X" not "Mac OSX"

Remember, it was "MacOS" once and "X" is actually the version number (roman 10) ;-)

> If I read the patch correct, then on Linux (which has no other AB types apart
> from mork & LDAP), the dialog will always show "Personal Address Book", I'm not
> sure it really needs to do that.

It might make sense (if someone does the coding) to add integration with the contact managers of the KDE or GNOME default PIM solutions though.
(In reply to comment #6)
> - Change the label for outlook/oe/OS X address books, prefixing it with "Link
> to ", i.e. "Link to Mac OS X Address Book".
I was considering using "Link to" too, but I thought it made it too wordy.

> btw its "Mac OS X" not "Mac OSX"
Oops!

> If I read the patch correct, then on Linux (which has no other AB types apart
> from mork & LDAP), the dialog will always show "Personal Address Book", I'm not
> sure it really needs to do that.
I can't easily hide the radio aspect, and I wanted to keep the label.

> Is this patch set up not to display Outlook/OE address books if those apps
> aren't install/set as default? I thought you were going to do that and I think
> you should.
My understanding is that the OE address book is always installed (in fact it's stored in a .wab file, meaning Windows Address Book.) Fortunately one of my PCs doesn't have Outlook on it, which will let me check that case.

> Also as it currently stands this patch will break Thunderbird as we share the
> dialog. If Bryan is happy with the possible changes then I'm happy to include
> the Mac part in Thunderbird at least.
Whoops, I forgot about that. Sorry.

> I'm not convinced the Outlook/OE code is really up to production standard with
> there being a requirement for it to be set up as the default application.
The Outlook code should work with any MAPI address book, although I know of no others (maybe SeaMonkey could read Thunderbird's addresses via MAPI?)
OK, so it looks like the OE address book is always available in Windows, but the Outlook address book cannot be created if Outlook isn't installed.
Lack of the OS X address book is causing  make check  to fail in mailnews/addrbook.  test_basic_nsIAbDirectory.js and test_nsAbManager2.js are the failing routines.

I'd sure like to see this enabled by default in SM, in some manner.   I must admit to not messing with the native address book, but will do so to test with SM.
Firstly, I think we should enable native addressbooks by default on all platforms which provide them. 

Secondly, I'm minussing both patches:
Showing/Hiding the native addressbooks is not likely to be changed often, thus a menuitem is inappropriate. 

In fact, this is a mere configuration setting which belongs onto a preference panel, including UI for recreating any deleted ab links (or even provide a checkmark list of native addressbooks). 

Hence the File->New dialog is inappropriate as well.
Attachment #324138 - Flags: review?(mnyromyr) → review-
Attachment #324802 - Flags: review?(mnyromyr) → review-
Not assuming that TB wants this, but if they do I'll update the patch :-)
Attachment #324138 - Attachment is obsolete: true
Attachment #324802 - Attachment is obsolete: true
Attachment #328115 - Flags: review?(mnyromyr)
Attachment #324138 - Flags: superreview?(neil)
Attachment #328115 - Flags: review?(mnyromyr) → review+
Attachment #328115 - Flags: review?(bugzilla)
Comment on attachment 328115 [details] [diff] [review]
Backend patch for comment 11

I'm happy with this as-is. I'm not prepared to enable the outlook interfaces on TB until I've done some investigation on how good they are etc (and the default client issue), and there's no point in holding this up until I've done that.
Attachment #328115 - Flags: review?(bugzilla) → review+
The dialog looks like it might be a good place to start if we had linking to other addressbooks like the OE and/or Linux.  Also we should look at (perhaps in another bug) the whole scenario for the windows address book, I'm not really aware of the user benefits for linking that as much as I am for the Mac.  For now I think it'd be better to keep the OS X link upfront and deal with multiple systems later.
Depends on: 448859
Depends on: 452939
As we even have some part of patch here, what's the progress on this?
Flags: wanted-seamonkey2?
(In reply to comment #15)
> As we even have some part of patch here, what's the progress on this?

AFAIK SM needs to agree a UI. TB has a menu option stowed away for the time being. If SM doesn't have a UI, and someone deletes the appropriate AB, then they won't be able to get it back again.
Flags: wanted-seamonkey2? → wanted-seamonkey2+
Whiteboard: [l10n-impact]
Not wanted for 2.0 any more at this stage, let's push it to 2.1
Flags: wanted-seamonkey2.1+
Flags: wanted-seamonkey2.0-
Flags: wanted-seamonkey2.0+
Assignee: mail → nobody
QA Contact: address-book → addressbook
Flags: wanted-seamonkey2.1+
Comment on attachment 328115 [details] [diff] [review]
Backend patch for comment 11

neil, please update patch and lets drive this into 2.2//2.3
Might make 2.3/2.4, missed the 2.2 boat though.
If the whiteboard is correct and this does in fact have l10n impact, it won't land on any branches.
You need to log in before you can comment on or make changes to this bug.