Read-only address book cards should have just a ok button that doesn't try to update the card

VERIFIED FIXED

Status

MailNews Core
Address Book
VERIFIED FIXED
12 years ago
9 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

({fixed-seamonkey1.0, fixed-seamonkey1.1a, fixed1.8.1})

Trunk
fixed-seamonkey1.0, fixed-seamonkey1.1a, fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

12 years ago
Currently, when we display read-only cards (i.e. LDAP address books) we display
them with both OK and Cancel buttons. This should really be just a close button.

At the moment when we click ok, it's accepted but not acted on due to
nsAbLDAPCard
(http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/src/nsAbLDAPCard.cpp#52)
which returns just NS_OK for the edit function and doesn't do anything with it.

However, bug 64305 calls for processing on the OK button to check the email
address is valid. If it is not valid it will put a dialog up which in the case
of read-only ldap books, is pretty pointless and confusing to the user.

My proposal is to replace the ok & cancel buttons with close for a read-only
card and return NS_ERROR_NOT_IMPLEMENTED in nsAbLDAPCard. Whilst that
effectively makes nsAbLDAPCard redundant, I'm guess Dan (or whoever) may want to
use it when we implement writeable LDAP directories, and hence I think we'll
leave it in for now.
(Assignee)

Comment 1

12 years ago
Created attachment 195038 [details] [diff] [review]
SeaMonkey only version 1

This works, but I'm not sure its the best way to do it.
(Assignee)

Comment 2

12 years ago
Created attachment 195285 [details] [diff] [review]
SeaMonkey only version 2

Version 2 of the patch, this reuses the cancel button as a close button.
Attachment #195038 - Attachment is obsolete: true
Attachment #195285 - Flags: superreview?(neil.parkwaycc.co.uk)
(Assignee)

Comment 3

12 years ago
Created attachment 195286 [details] [diff] [review]
SeaMonkey only version 3

Version 3 - slightly different, drop the cancel button, keeping the ok, but
re-mapping it to do the same as cancel.

Take your pick or suggest a different method ;-)
Attachment #195286 - Flags: superreview?(neil.parkwaycc.co.uk)
(Assignee)

Comment 4

12 years ago
Comment on attachment 195285 [details] [diff] [review]
SeaMonkey only version 2

We're going for version 3a.
Attachment #195285 - Attachment is obsolete: true
Attachment #195285 - Flags: superreview?(neil.parkwaycc.co.uk)
(Assignee)

Updated

12 years ago
Attachment #195286 - Flags: superreview?(neil.parkwaycc.co.uk)
(Assignee)

Comment 5

12 years ago
Created attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

We are going with the single ok button version (as discussed with Neil on irc).


Changes from previous version: removed unnecessary setting of default button
attribute, also changed setting of ondialogaccept to removal of that attribute
to avoid the one set in the xul.
Attachment #195286 - Attachment is obsolete: true
Attachment #196317 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #196317 - Flags: review?(neil.parkwaycc.co.uk)
(Assignee)

Comment 6

12 years ago
Created attachment 196448 [details] [diff] [review]
Thunderbird version (checked in)

Thunderbird version of the patch.
Attachment #196448 - Flags: superreview?(bienvenu)
Attachment #196448 - Flags: review?(bienvenu)
(Assignee)

Comment 7

12 years ago
Updating summary from "Read-only address book cards should have just a close
button" to reflect what we are actually doing.
Summary: Read-only address book cards should have just a close button → Read-only address book cards should have just a ok button that doesn't try to update the card

Updated

12 years ago
Attachment #196448 - Flags: superreview?(bienvenu)
Attachment #196448 - Flags: superreview+
Attachment #196448 - Flags: review?(bienvenu)
Attachment #196448 - Flags: review+
(Assignee)

Comment 8

12 years ago
Comment on attachment 196448 [details] [diff] [review]
Thunderbird version (checked in)

/cvsroot/mozilla/mail/components/addrbook/content/abCardOverlay.js,v  <-- 
abCardOverlay.js
new revision: 1.9; previous revision: 1.8
Attachment #196448 - Attachment description: Thunderbird version → Thunderbird version (checked in)

Updated

12 years ago
Attachment #196317 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #196317 - Flags: superreview+
Attachment #196317 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #196317 - Flags: review+
(Assignee)

Comment 9

12 years ago
With this change we should also now change nsAbLDAPCard::EditCardToDatabase to 
return NS_ERROR_NOT_IMPLEMENTED instead of NS_OK 
(http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/src/nsAbLDAPCard.cpp#
54).

It was done because we were incorrectly trying to edit the card.

Now we don't do that we should return NS_ERROR_NOT_IMPLEMENTED until we get 
round to implementing writable LDAP directories. As we are planning on doing 
that then I don't see any real reason to remove nsAbLDAPCard at the moment.
Status: NEW → ASSIGNED
(Assignee)

Comment 10

12 years ago
Comment on attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

/cvsroot/mozilla/mailnews/addrbook/resources/content/abCardOverlay.js,v  <-- 
abCardOverlay.js
new revision: 1.56; previous revision: 1.55
/cvsroot/mozilla/mailnews/addrbook/resources/content/abEditCardDialog.xul,v 
<--  abEditCardDialog.xul
new revision: 1.21; previous revision: 1.20
Attachment #196317 - Attachment description: SeaMonkey only version 3a → SeaMonkey only version 3a (checked in)
(Assignee)

Comment 11

12 years ago
Created attachment 196814 [details] [diff] [review]
Follow up patch (checked in)

This follow up patch changes NS_OK to NS_ERROR_NOT_IMPLEMENTED in
nsAbLDAPCard::EditCardToDatabase. Now we're not trying to edit cards, we can
return the "correct" value for the current code. As I said before, I think we
should leave nsAbLDAPCard in until we get a better idea on what we are doing
about writeable ldap directories.
Attachment #196814 - Flags: superreview?(bienvenu)
Attachment #196814 - Flags: review?(bienvenu)

Updated

12 years ago
Attachment #196814 - Flags: superreview?(bienvenu)
Attachment #196814 - Flags: superreview+
Attachment #196814 - Flags: review?(bienvenu)
Attachment #196814 - Flags: review+
(Assignee)

Comment 12

12 years ago
Comment on attachment 196814 [details] [diff] [review]
Follow up patch (checked in)

/cvsroot/mozilla/mailnews/addrbook/src/nsAbLDAPCard.cpp,v  <-- 
nsAbLDAPCard.cpp
new revision: 1.6; previous revision: 1.5
Attachment #196814 - Attachment description: Follow up patch → Follow up patch (checked in)
(Assignee)

Updated

12 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Verified FIXED using version 1.6a1 (20051022) of Thunderbird and SeaMonkey 1.1a;Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051023 Mozilla/1.0 with prefs:

Hostname: ads.iu.edu
Base DN: dc=ads,dc=iu,dc=edu
Port number: 3269
Bind DN: ads\stdonner
[X} Use secure connection (SSL) checked

Address book LDAP cards are read-only, and are presented with grayed-out fields, and a single "Close" button.

Great work, Mark!
Status: RESOLVED → VERIFIED
(Assignee)

Comment 14

12 years ago
Comment on attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

Requesting approval for checkin on branch of SeaMonkey-only patch.
Attachment #196317 - Flags: approval1.8.0.1?
Attachment #196317 - Flags: approval-aviary1.0.8?
(Assignee)

Comment 15

12 years ago
Comment on attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

Opps, didn't mean to set 1.0.8?, DID mean to set 1.8.0.1?
Attachment #196317 - Flags: approval-aviary1.0.8?
Comment on attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

a=ctho, just need one more

Comment 17

12 years ago
Comment on attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

approval-seamonkey1.0=me
(Assignee)

Comment 18

12 years ago
Comment on attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

Cancelling obsolete review request, approval-seamonkey1.0 given by SeaMonkey Council (see above 2 comments).
Attachment #196317 - Flags: approval1.8.0.1?
(Assignee)

Comment 19

12 years ago
Comment on attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

I just checked this in the 1.8 branch for SeaMonkey1.0
Attachment #196317 - Attachment description: SeaMonkey only version 3a (checked in) → SeaMonkey only version 3a (checked in, trunk and branch)
(Assignee)

Updated

12 years ago
Whiteboard: SeaMonkey only fixed1.8

Updated

12 years ago
Whiteboard: SeaMonkey only fixed1.8 → fixed-seamonkey1.0
(Assignee)

Comment 20

12 years ago
Comment on attachment 196317 [details] [diff] [review]
SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)

Checked in to 1.8.0 branch:
Checking in mailnews/addrbook/resources/content/abCardOverlay.js;
/cvsroot/mozilla/mailnews/addrbook/resources/content/abCardOverlay.js,v  <--  abCardOverlay.js
new revision: 1.55.8.1; previous revision: 1.55
done
Checking in mailnews/addrbook/resources/content/abEditCardDialog.xul;
/cvsroot/mozilla/mailnews/addrbook/resources/content/abEditCardDialog.xul,v  <--  abEditCardDialog.xul
new revision: 1.20.30.1; previous revision: 1.20
Attachment #196317 - Attachment description: SeaMonkey only version 3a (checked in, trunk and branch) → SeaMonkey only version 3a (checked in, trunk and 1.8 and 1.8.0 branches)
(Assignee)

Comment 21

12 years ago
Comment on attachment 196448 [details] [diff] [review]
Thunderbird version (checked in)

Requesting approval for 1.8.1 branch. Simple patch to improve ldap address book card dialogs. This will be needed to allow bug 64305 & others into the branch that we'll want for trunk synchronisation for birthday & anniversary fields if we implement them.
Attachment #196448 - Flags: approval1.8.1?

Updated

12 years ago
Attachment #196448 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Updated

12 years ago
Keywords: fixed-seamonkey1.0, fixed-seamonkey1.1a, fixed1.8.1
Whiteboard: fixed-seamonkey1.0
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.