[autoconfig] Pass Exchange URL from account creation dialog to addon

RESOLVED FIXED in Thunderbird 66.0

Status

enhancement
P3
normal
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: BenB, Assigned: BenB)

Tracking

(Blocks 1 bug)

Trunk
Thunderbird 66.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird_esr6065+ fixed, thunderbird65 fixed, thunderbird66 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

5 months ago

The account creation dialog runs the Exchange AutoDiscover methods and finds the Exhange protocol URLs (OWA and EWS URL). It then extracts the hostname from it, creates a normal nsIMsgIncomingServer and throws away the URLs. Owl the synthezises the URL from the hostname again. Obviously, that's information loss. Even though it happens to work in all cases we know, it's still not smart.

The account creation wizard, when creating the account, should set the preferences <server>.owa_url, <server>.ews_url and <server>.eas_url , if these URLs have been found during AutoDiscover. The Exchange addon can then use these URLs to contact the server.

Assignee

Comment 1

5 months ago
Posted patch Fix, v2 (obsolete) — Splinter Review

This also fixes up the existing POP3 code which was ...unnecessarily long (even when not considering the nsIMsgIncomingServer.set*Pref() API).

Assignee: nobody → ben.bucksch
Status: NEW → ASSIGNED
Attachment #9035418 - Flags: review?(neil)
Assignee

Comment 2

5 months ago
Posted patch Fix, v3 (obsolete) — Splinter Review

Caught a leftover during self-review.

Attachment #9035418 - Attachment is obsolete: true
Attachment #9035418 - Flags: review?(neil)
Attachment #9035420 - Flags: review?(neil)

Comment on attachment 9035420 [details] [diff] [review]
Fix, v3

+  if (config.incoming.owaURL) {
+    inServer.setCharValue("owa_url", config.incoming.owaURL);
+  }
+  if (config.incoming.ewsURL) {
+    inServer.setCharValue("ews_url", config.incoming.ewsURL);
+  }
+  if (config.incoming.easURL) {
+    inServer.setCharValue("eas_url", config.incoming.easURL);
  }

Please use setUnicharValue in case the URL contains Unicode. r=me with that fixed.

Attachment #9035420 - Flags: review?(neil) → review+
Assignee

Comment 4

4 months ago
Posted patch Fix, v4Splinter Review
  • Use setUnicharValue()
Attachment #9035420 - Attachment is obsolete: true
Attachment #9035652 - Flags: review?(neil)

Updated

4 months ago
Attachment #9035652 - Flags: review?(neil) → review+
Assignee

Comment 5

4 months ago

User impact if declined:
Minimal. This affects only users where the OWA URL is not https://host/owa/, but e.g. https://host/OWA/. Even in these cases, /owa/ might be redirected to /OWA/ and work anyway, just a tiny little slower due to the redirect. So, this is mostly a correctness fix.
Developer impact if declined:
Other autoconfig patches might be harder to backport later.
Risk:
Affects Owl and some advanced POP3 settings. Behavior for the POP3 should not change, just the code is streamlined.

Assignee

Updated

4 months ago
Priority: -- → P3

Comment 6

4 months ago

Why the need for tracking? Request beta uplift on the patch.

Target Milestone: --- → Thunderbird 66.0

Comment 7

4 months ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/1ff4cbc59aae
[autoconfig] Pass Exchange URL from account creation dialog to addon. r=Neil

Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Keywords: checkin-needed
Resolution: --- → FIXED

Comment 8

4 months ago

Ben, can you please request uplifts as required. I am constantly monitoring potential uplifts and the sooner you request, the better, avoids missing the next uplift I'm doing. Currently I have like five bugs, so it's going to happen soon.

Assignee

Comment 9

4 months ago

Comment on attachment 9035652 [details] [diff] [review]
Fix, v4

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: Minimal. This affects only users where the OWA URL is not https://host/owa/, but e.g. https://host/OWA/. Even in these cases, /owa/ might be redirected to /OWA/ and work anyway, just a tiny little slower due to the redirect. So, this is mostly a correctness fix.

Developer impact if declined:
Other autoconfig patches might be harder to backport later.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce:

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Affects Owl and some advanced POP3 settings. Behavior for the POP3 should not change, just the code is streamlined.

String changes made/needed:

Attachment #9035652 - Flags: approval-mozilla-beta?
Assignee

Comment 10

4 months ago

oops, some of the fields defaulted to wrong values.

Assignee

Comment 11

4 months ago

Comment on attachment 9035652 [details] [diff] [review]
Fix, v4

Sorry, I'm totally confused about all these flags in different places. (I do understand the differences, I was just confused.) Sorry for the noise. This should be the right one.

[Approval Request Comment]
See comment 5

Attachment #9035652 - Flags: approval-mozilla-beta?
Attachment #9035652 - Flags: approval-comm-esr60?
Attachment #9035652 - Flags: approval-comm-beta?

Updated

4 months ago
Attachment #9035652 - Flags: approval-comm-beta? → approval-comm-beta+

Comment 13

4 months ago
Comment on attachment 9035652 [details] [diff] [review]
Fix, v4

Using rebased patch from Neil's try. Same as this one with white-space adjustments.
Attachment #9035652 - Flags: approval-comm-esr60? → approval-comm-esr60+
You need to log in before you can comment on or make changes to this bug.