Closed Bug 1518890 Opened 4 years ago Closed 4 years ago

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

Categories

(Thunderbird :: Account Manager, enhancement, P3)

enhancement

Tracking

(thunderbird_esr6065+ fixed, thunderbird65 fixed, thunderbird66 fixed)

RESOLVED FIXED
Thunderbird 66.0
Tracking Status
thunderbird_esr60 65+ fixed
thunderbird65 --- fixed
thunderbird66 --- fixed

People

(Reporter: BenB, Assigned: BenB)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.

Attached 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)
Attached 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+
Attached patch Fix, v4Splinter Review
  • Use setUnicharValue()
Attachment #9035420 - Attachment is obsolete: true
Attachment #9035652 - Flags: review?(neil)
Attachment #9035652 - Flags: review?(neil) → review+

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.

Priority: -- → P3

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

Target Milestone: --- → Thunderbird 66.0

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
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

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.

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?

oops, some of the fields defaulted to wrong values.

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?
Attachment #9035652 - Flags: approval-comm-beta? → approval-comm-beta+
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.