Closed Bug 1560307 Opened 4 months ago Closed Last month

initServerType throws an exception for non-built-in server types

Categories

(Thunderbird :: Account Manager, enhancement, minor)

enhancement
Not set
minor

Tracking

(thunderbird_esr6869+ fixed, thunderbird70 fixed, thunderbird71 fixed)

RESOLVED FIXED
Thunderbird 71.0
Tracking Status
thunderbird_esr68 69+ fixed
thunderbird70 --- fixed
thunderbird71 --- fixed

People

(Reporter: neil, Assigned: aceman)

Details

Attachments

(1 file)

The page wants to look up the pretty name of the server type here:

  var serverType = document.getElementById("server.type").getAttribute("value");
  var propertyName = "serverType-" + serverType;

  var messengerBundle = document.getElementById("bundle_messenger");
  var verboseName = messengerBundle.getString(propertyName);
  setDivText("servertype.verbose", verboseName);

(Owl currently monkeypatches this function anyway in order to adjust the authentication menulist, so we just hard code this to "owl", while ExQuilla doesn't use this page.)

Could you maybe somehow add the needed string to the messenger bundle? Or is that not possible at runtime?

I don't think you can do that at runtime; the string bundle API doesn't have any methods for writing, and you obviously can't write to the locale files.

Attached patch 1560307.patchSplinter Review

OK, we can catch the case in the AM code. Do you see a way the addon can provide that type description?
Theoretically, displaying the raw type is OK in many cases as it is already prefixed with a label "Server type:" so if it comes out as "Server type: nntp", it is understandable.
But e.g. for Owl addon it would display "Server type: owl" (if you didn't patch the function), which surely isn't the best presentation. You probably would want to display something like "MS Exchange (provided by Owl)".

Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #9074317 - Flags: feedback?(neil)
Comment on attachment 9074317 [details] [diff] [review]
1560307.patch

... actually although we patch the function we just use the raw type anyway; I guess we should change this...
Attachment #9074317 - Flags: feedback?(neil) → feedback+
Attachment #9074317 - Flags: review?(jorgk)
Comment on attachment 9074317 [details] [diff] [review]
1560307.patch

Must be good if Neil likes it. Any uplift for this to ESR 68? I'll stick it into the beta.
Attachment #9074317 - Flags: review?(jorgk)
Attachment #9074317 - Flags: review+
Attachment #9074317 - Flags: approval-comm-esr68?
Attachment #9074317 - Flags: approval-comm-beta+
Keywords: checkin-needed

The only user is probably Owl, and they have it overridden, so there shouldn't be any harm to get it into 68. Owl can then choose if they take advantage of it in any way.

Target Milestone: --- → Thunderbird 71.0

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/dd1f25a42329
Handle missing server type description for addon-provided server types in account manager. r=jork DONTBUILD

Status: ASSIGNED → RESOLVED
Closed: Last month
Keywords: checkin-needed
Resolution: --- → FIXED
Attachment #9074317 - Flags: approval-comm-esr68? → approval-comm-esr68+
You need to log in before you can comment on or make changes to this bug.