Create test for mailbox URLs

RESOLVED FIXED in Thunderbird 68.0

Status

enhancement
RESOLVED FIXED
a month ago
29 days ago

People

(Reporter: jorgk, Assigned: jorgk)

Tracking

Trunk
Thunderbird 68.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a month ago

+++ This bug was initially created as a clone of Bug #1533320 +++

Quote from 1533320 comment #23:

A simple xpcshell-test should suffice. Similar to netwerk/test/unit/test_URIs.js or netwerk/test/unit/test_standardurl.js
Use Services.io.newURI to create a few URIs of each type, and make sure that setting the username for mailbox URIs throws without triggering the assert.

(Assignee)

Updated

a month ago
Component: General → General
OS: Linux → All
Product: Thunderbird → MailNews Core
Hardware: x86_64 → All
(Assignee)

Comment 1

a month ago

This should do it. Any idea how to handle the Windows path names better?

Digging through my bug mail, I can see that mailbox URLs look like this:
mailbox:///C:/Users/jorgk/AppData/Roaming/Thunderbird/Profiles/5gfhfw7f.POP%20tbtest/Mail/server.de/Inbox?number=1
So there are always three slashes after the mailbox:, and as we know, the code checks that on various occasions.

Let's see what the other platforms think:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9555cb3e0f19f5ef9d7792bc444cf6e74543356b

I started with https://searchfox.org/comm-central/source/ldap/xpcom/tests/unit/test_nsLDAPURL.js and the dumps are also from there.

Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #9052969 - Flags: review?(valentin.gosu)
(Assignee)

Comment 2

29 days ago

Test passed on Linux, Mac currently busted. Fix for linting errors.

Attachment #9052969 - Attachment is obsolete: true
Attachment #9052969 - Flags: review?(valentin.gosu)
Attachment #9052998 - Flags: review?(valentin.gosu)
Comment on attachment 9052998 [details] [diff] [review]
1535964-mailbox-URL-test.patch (v1b)

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

::: mailnews/local/test/unit/test_mailboxURL.js
@@ +6,5 @@
> +var mailboxFile = Services.dirsvc.get("TmpD", Ci.nsIFile);
> +mailboxFile.append("mailFolder");
> +mailboxFile.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0o700);
> +// Replace backslashes on Windows.
> +var mailboxFileName = "/" + mailboxFile.path.replace(/\\/g, "/");

You can try something like `let mailboxFileName = Services.io.newFileURI(mailboxFile).pathQueryRef;`

@@ +46,5 @@
> +
> +  // Test - get and check urls.
> +  var part = 0;
> +  for (part = 0; part < mailboxURLs.length; part++) {
> +    dump("url: " + mailboxURLs[part].url + "\n");

```dump(`url: ${mailboxURLs[part].url}\n`); ```

looks way better
Attachment #9052998 - Flags: review?(valentin.gosu) → review+

Comment 4

29 days ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3496ec1acebe
create test for mailbox: URLs. r=valentin

Status: ASSIGNED → RESOLVED
Last Resolved: 29 days ago
Resolution: --- → FIXED
(Assignee)

Comment 5

29 days ago

Thanks, Valentin. I landed it with the two suggestions you made.

Target Milestone: --- → Thunderbird 68.0
You need to log in before you can comment on or make changes to this bug.