Closed Bug 1389173 Opened 4 years ago Closed 3 years ago

Implement nsLDAPURL::GetDisplay[Spec|Host|HostPort] better and enhance test

Categories

(MailNews Core :: LDAP Integration, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 63.0

People

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

References

Details

Attachments

(1 file, 1 obsolete file)

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

In bug 1388927 I had to adjust the test expectation
https://hg.mozilla.org/comm-central/rev/430f8faf075db3d4915786d534b811530267fb23#l1.12
since spec, host, hostPort and prePath now return (ASCII) punycode due to bug 1380617.

Since the test contains a URL with Japanese characters we should consider implementing nsLDAPURL::GetDisplay[Spec|Host|HostPort] and restore the test case.
Attached patch 1389173-ldap-url.patch (v1) (obsolete) — Splinter Review
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8895934 - Flags: review?(rkent)
Looking at bug 1389762, we might need to do something like this for NNTP as well.
Please ignore the previous comment, bug 1389762 has a different cause.
Assignee: jorgk → nobody
Status: ASSIGNED → NEW
Valentin, do you think it makes sense to "improve" the implementations of GetDisplay[Spec|Host|HostPort] or should this bug just be a WONTFIX?
Attachment #8990458 - Flags: review?(acelists)
Attachment #8990458 - Flags: feedback?(valentin.gosu)
Attachment #8895934 - Attachment is obsolete: true
Attachment #8895934 - Flags: review?(rkent)
Can we use any of these Display* attributes in the base code?

Somehow we have a lot of reported problems (in TB52) due to urls being changed to punycode internally even before bug 1388927.
We still couldn't solve them :(
(In reply to :aceman from comment #5)
> Can we use any of these Display* attributes in the base code?
What do you mean?

> Somehow we have a lot of reported problems (in TB52) due to urls being
> changed to punycode internally even before bug 1388927.
Reported where?
Attachment #8990458 - Flags: feedback?(valentin.gosu) → feedback+
(In reply to Jorg K (GMT+2) from comment #6)
> (In reply to :aceman from comment #5)
> > Can we use any of these Display* attributes in the base code?
> What do you mean?

If we call these properties anywhere in TB. It seems not. Only the test uses them now.
But as those attributes are defined on the parent classes we should have them report correct values (decoded).

> > Somehow we have a lot of reported problems (in TB52) due to urls being
> > changed to punycode internally even before bug 1388927.
> Reported where?

Like bug 1369343 and bug 1360836. I have debugged those locally and it is because at some point m-c coverts the international server names to punycode which we do not expect and then we can no longer find any folders for the server.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8990458 - Flags: review?(acelists) → review+
(In reply to :aceman from comment #7)
> Like bug 1369343 and bug 1360836. I have debugged those locally and it is
> because at some point m-c coverts the international server names to punycode
> which we do not expect and then we can no longer find any folders for the
> server.
And we haven't fixed those yet :-(
Should we consider implementing better GetDisplay[Spec|Host|HostPort|PrePath] for the other three URLs we have: mailnews, address book and mailto? See: https://hg.mozilla.org/comm-central/rev/4c712bcd9062.

For LDAP this bug here was motivated since I had to change a test here:
https://hg.mozilla.org/comm-central/rev/430f8faf075d
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/a75878fe1f57
Implement better GetDisplay[Spec|Host|HostPort|PrePath] for LDAP and enhance related test. r=aceman
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 63.0
You need to log in before you can comment on or make changes to this bug.