Last Comment Bug 522342 - Allow JS/UI code to list other users with access to the folder (list full ACL)
: Allow JS/UI code to list other users with access to the folder (list full ACL)
Status: RESOLVED FIXED
: fixed-seamonkey2.0.1
Product: MailNews Core
Classification: Components
Component: Networking: IMAP (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: Thunderbird 3.0rc1
Assigned To: Ben Bucksch (:BenB)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-14 14:00 PDT by Ben Bucksch (:BenB)
Modified: 2009-12-13 06:05 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix, v1 (5.70 KB, patch)
2009-10-14 14:01 PDT, Ben Bucksch (:BenB)
no flags Details | Diff | Review
Fix, v2 (6.68 KB, patch)
2009-10-14 15:31 PDT, Ben Bucksch (:BenB)
mozilla: review+
Details | Diff | Review
Fix, v3 - new fresh yummy yummy, up-to-date to trunk (6.68 KB, patch)
2009-10-15 16:29 PDT, Ben Bucksch (:BenB)
mozilla: superreview+
mozilla: approval‑thunderbird3+
Details | Diff | Review
Fix, v4 (6.71 KB, patch)
2009-10-15 16:50 PDT, Ben Bucksch (:BenB)
no flags Details | Diff | Review
Fix crash in TB on m-c (1.29 KB, patch)
2009-10-16 07:13 PDT, Ben Bucksch (:BenB)
standard8: review+
standard8: superreview+
standard8: approval‑thunderbird3+
Details | Diff | Review

Description Ben Bucksch (:BenB) 2009-10-14 14:00:10 PDT
I'd like to list the whole access list (ACL) of a folder in the folder properties dialog, including which other users have access to this folder and which rights each has.

We have this information in the backend, we just don't use it and don't have an IDL for it. This bug asks to implement that.

Attaching patch.
Comment 1 Ben Bucksch (:BenB) 2009-10-14 14:01:01 PDT
Created attachment 406295 [details] [diff] [review]
Fix, v1
Comment 2 David :Bienvenu 2009-10-14 14:08:45 PDT
Comment on attachment 406295 [details] [diff] [review]
Fix, v1

looks good in general.

need to rev the uuid.

+   * List all (human) users apart from the current user which have access to

s/which/who

+   * this folder. (List is not guaranteed to be complete.)

When will the list not be complete?
Comment 3 Ben Bucksch (:BenB) 2009-10-14 14:16:55 PDT
> need to rev the uuid.

Done

> s/which/who

Done

> > (List is not guaranteed to be complete.)
> When will the list not be complete?

I don't know. I just didn't know whether the code guarantees it. I can remove that sentence.
Done

I also revised the description of the permission flags.
Comment 4 Ben Bucksch (:BenB) 2009-10-14 15:31:32 PDT
Created attachment 406325 [details] [diff] [review]
Fix, v2

Fixed above feedback.

I had to do a full rebuild (because of the uuid rev) - make clean && make in mailnews/ and mail/ were not sufficient :(.
Comment 5 Ben Bucksch (:BenB) 2009-10-14 15:32:15 PDT
(before that, I got XPCOM errors when calling the new functions)
Comment 6 David :Bienvenu 2009-10-14 15:34:24 PDT
make -s tier_app in the obj-dir always does the trick for me...
Comment 7 David :Bienvenu 2009-10-15 08:19:13 PDT
Comment on attachment 406325 [details] [diff] [review]
Fix, v2

the patch is bit-rotted - it seems to have been made against a slightly out of date tree.

these two lines are a bit long - can you wrap them?

+static PLDHashOperator fillArrayWithKeys(const nsACString& key, const nsCString data, void* userArg)

+nsresult nsImapMailFolder::GetOtherUsersWithAccess(nsIUTF8StringEnumerator** aResult)

this needs to be NS_IMETHODIMP, not nsresult:

+nsresult nsImapMailFolder::GetPermissionsForUser(const nsACString& otherUser, nsACString& aResult)

r=me, with those fixed.
Comment 8 Ben Bucksch (:BenB) 2009-10-15 16:29:44 PDT
Created attachment 406572 [details] [diff] [review]
Fix, v3 - new fresh yummy yummy, up-to-date to trunk
Comment 9 David :Bienvenu 2009-10-15 16:44:22 PDT
Comment on attachment 406572 [details] [diff] [review]
Fix, v3 - new fresh yummy yummy, up-to-date to trunk

this still needs to be NS_IMETHODIMP

+nsresult nsImapMailFolder::GetOtherUsersWithAccess(nsIUTF8StringEnumerator** aResult)

and this should be wrapped:

+static PLDHashOperator fillArrayWithKeys(const nsACString& key, const nsCString data, void* userArg)


sr/a=me, with those comments addressed.
Comment 10 Ben Bucksch (:BenB) 2009-10-15 16:50:07 PDT
Created attachment 406580 [details] [diff] [review]
Fix, v4

Ops, sorry. Fixed.
Comment 11 Ben Bucksch (:BenB) 2009-10-15 19:35:54 PDT
Commited http://hg.mozilla.org/comm-central/rev/81f036d91636

Thunderbird on mozilla-central trunk
http://tinderbox.mozilla.org/showbuilds.cgi?tree=Thunderbird
is burning, though, because somebody decided to remove nsCStringArray (I just followed https://developer.mozilla.org/en/XPCOM_array_guide ).
Thunderbird 3.0 on Mozilla 1.9.1 is fine
http://build.mozillamessaging.com/tinderboxpushlog/?tree=Thunderbird3.0
so I'll care about this tomorrow.
Comment 12 Serge Gautherie (:sgautherie) 2009-10-15 22:28:37 PDT
(In reply to comment #11)
> so I'll care about this tomorrow.

Please, at least, tag the (SM & TB) builds...
Comment 13 Ben Bucksch (:BenB) 2009-10-15 22:50:44 PDT
Fixed, hopefully, in http://hg.mozilla.org/comm-central/rev/5b94dffb67e4

The NS_NewAdoptingUTF8StringEnumerator() API changed its second parameter from nsCStringArray in 1.9.1 to nsTArray<nsCString> in m-c, so I used an #ifdef.
Comment 14 Ben Bucksch (:BenB) 2009-10-16 07:13:39 PDT
Created attachment 406677 [details] [diff] [review]
Fix crash in TB on m-c

The bustage fix was incomplete, I need to adapt the cast to the new array type as well. Without this patch, this causes TB on m-c to crash when accessing the new functions added here.
Comment 15 Ben Bucksch (:BenB) 2009-10-16 12:05:30 PDT
Comment on attachment 406677 [details] [diff] [review]
Fix crash in TB on m-c

Checked in as http://hg.mozilla.org/comm-central/rev/3e7f71a517bf
Comment 16 Ben Bucksch (:BenB) 2009-10-16 12:06:17 PDT
Comment on attachment 406677 [details] [diff] [review]
Fix crash in TB on m-c

Sorry, I meant http://hg.mozilla.org/comm-central/rev/b887790ca1f2
Comment 17 Ben Bucksch (:BenB) 2009-10-19 13:09:46 PDT
FIXED

Note You need to log in before you can comment on or make changes to this bug.