Closed Bug 1494238 Opened Last year Closed Last year

Replace class AdoptUTF8StringEnumerator by using NS_NewAdoptingUTF8StringEnumerator()

Categories

(MailNews Core :: Networking: IMAP, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 64.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

As per IRC:

10:09:26 - darktrojan: darktrojan> jorgk, I don't think the class AdoptUTF8StringEnumerator is needed now
10:09:27 - darktrojan: <darktrojan> there's NS_NewAdoptingUTF8StringEnumerator
Summary: Replace class AdoptUTF8StringEnumerator and use NS_NewAdoptingUTF8StringEnumerator() → Replace class AdoptUTF8StringEnumerator by using NS_NewAdoptingUTF8StringEnumerator()
IRC:
10:09:27 - darktrojan: <darktrojan> but that's just my uneducated look at it

Now the reviewer can take another educated look at it ;-)
Assignee: nobody → jorgk
Attachment #9012095 - Flags: review?(geoff)
Comment on attachment 9012095 [details] [diff] [review]
1494238-AdoptUTF8StringEnumerator.patch

In case I have no better patches to land, perhaps Aceman can review this.
Attachment #9012095 - Flags: review?(acelists)
Comment on attachment 9012095 [details] [diff] [review]
1494238-AdoptUTF8StringEnumerator.patch

Review of attachment 9012095 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting. The code in m-c seems to do the same.
Let's hope they do not remove the class soon, there are very few uses in m-c. But then if they migrate to something else, we should be able to do the same.
Attachment #9012095 - Flags: review?(acelists) → review+
I've added this in am-server.js where you select an IMAP trash folder:
function folderPickerChange(aEvent)
{
  var folder = aEvent.target._folder;
  let users = folder.QueryInterface(Ci.nsIMsgImapMailFolder).getOtherUsersWithAccess();
  for (u of users) dump(`=== here is a user ${u}\n`);

It prints OK, then crashes. Next I added the missing NS_ADDREF() like so:
  NS_ADDREF(*aResult = new AdoptUTF8StringEnumerator(resultArray));
and it no longer crashes.

So, two options: Use first patch that will call M-C code that has the addref and the function will work, or remove the dead faulty code.
Attachment #9012240 - Flags: feedback?(geoff) → feedback+
Comment on attachment 9012399 [details] [diff] [review]
GetOtherUsers.patch - patch for experimenting

Review of attachment 9012399 [details] [diff] [review]:
-----------------------------------------------------------------

I'm usually keen on getting rid of leftovers. But this seems like it would be a very nice addition in the folder properties Sharing tab.

Tried this patch but the only folder that it will show something on is the inbox (testing on fastmail). For the other folders there's no dump. The folderPickerChange is triggered but whatever errors follows are silently swallowed
Comment on attachment 9012240 [details] [diff] [review]
1494238-AdoptUTF8StringEnumerator.patch - complete removal

OK, yes, it could be a useful function to have in the box. Let's fix it with the other patch then. I'll test it again with the experimental patch.
Attachment #9012240 - Attachment is obsolete: true
Attachment #9012240 - Flags: review?(mkmelin+mozilla)
Attachment #9012240 - Flags: review?(acelists)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/102bf0b97b0a
Fix nsMsgIMAPFolderACL::GetOtherUsers() (missing addref) by replacing class AdoptUTF8StringEnumerator with using NS_NewAdoptingUTF8StringEnumerator(). r=aceman
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 64.0
You need to log in before you can comment on or make changes to this bug.