Closed Bug 1225935 Opened 9 years ago Closed 8 years ago

Use https as connection protocol for map lookups of addresses instead of insecure http

Categories

(Thunderbird :: Address Book, enhancement)

enhancement
Not set
normal

Tracking

(thunderbird45- affected, thunderbird46 fixed, thunderbird47 fixed)

RESOLVED FIXED
Thunderbird 47.0
Tracking Status
thunderbird45 - affected
thunderbird46 --- fixed
thunderbird47 --- fixed

People

(Reporter: aryx, Assigned: aceman)

Details

Attachments

(1 file)

https://dxr.mozilla.org/comm-central/rev/49a5d80772d926ab85b0a8cf265c72f877acb998/mail/locales/en-US/chrome/messenger-region/region.properties#26
and
https://dxr.mozilla.org/comm-central/rev/49a5d80772d926ab85b0a8cf265c72f877acb998/suite/locales/en-US/chrome/mailnews/region.properties#25
use the http protocol for looking the addresses of contacts in the addressbook.

These should be https urls. Google Maps uses https by default, for OSM which supports both http and https but doesn't upgrade the connection, it might be better to ask the OSM webops first.

Aceman, you worked on bug 531285, are you interested in this?
Yes, but we need to find a clever way to implement this. Currently if most of the users have the http version in their mail.addr_book.mapit_url.format, changing the current version of mail.addr_book.mapit_url.1.format to https (or any other change), will automatically cause creating of a new user pref of mail.addr_book.mapit_url.100.format where this old http copy gets stored.
We could afford that for the new OSM which I think is not in release yet (so probably nobody uses it yet). But for gmaps it is worse (as all users have it now).
Attached patch patchSplinter Review
OK, I have tested it and when users have not yet customized the mail.addr_book.mapit_url.format pref yet, they can be automatically upgraded (the new pref value (URL) is used) fine without creating the entries I described.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Comment on attachment 8696252 [details] [diff] [review]
patch

I have no idea why I forgot to request a review here.
Anyway, we should get it out in TB45 before many people get to use the old URLs.
Attachment #8696252 - Flags: review?(mkmelin+mozilla)
Attachment #8696252 - Flags: review?(iann_bugzilla)
Comment on attachment 8696252 [details] [diff] [review]
patch

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

r=mkmelin - you should probably post about this in m.d.l10n
Attachment #8696252 - Flags: review?(mkmelin+mozilla) → review+
Attachment #8696252 - Flags: review?(iann_bugzilla) → review+
Thanks.
Severity: normal → enhancement
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
https://hg.mozilla.org/comm-central/rev/4a60c0ffe9af
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 47.0
Comment on attachment 8696252 [details] [diff] [review]
patch

[Approval Request Comment]
This should be safe for branches as the string IDs are not changed. I'll post to l10n newsgroup for localizers that want to pick up this change.
Attachment #8696252 - Flags: approval-comm-beta?
Attachment #8696252 - Flags: approval-comm-aurora?
Attachment #8696252 - Flags: approval-comm-aurora? → approval-comm-aurora+
Questions and comment for TB 45.

First, I would not normally push a change to suite for comm-beta. Also, are they still CLOSED TREE? I will not push against a closed tree. If I push this, I will strip out the suite part unless specifically requested not to (and the suite tree is open).

Second, I am wondering whether this was the right approach this late. l10n tools are automated so this change will not get picked up. A post to l10n is less than optimal. Who can speak for l10n to say that pushing this to beta with no ID change and a post to the newsgroup is acceptable practice? (You could have for example created a new string ID, used it by default but silently used the old string if not available. Or automatically changed the http:// to https:// in the strings for default providers).
I know nothing about it, I pushed to Aurora as was requested. Or was that a mistake? I didn't see any strings that needed translation.
Pushing this to beta (45) is an attempt to never expose the http version of openstreetmap to users. If we let the http version in TB45 and any users start to use it, they will not be upgraded automatically to the https version later on. Actually, they will get a duplicate entry in the URL list one with the new https version and a "user-URL" with the old http version.

Yes, it is unfortunate this got so late in the cycle (lost time in comment 3:().

So of course it is your decision now whether it gets into 45. As for localizers, maybe ask Aryx how it works.
Flags: needinfo?(aryx.bugmail)
If the desire is to "never expose the http version of openstreetmap to users" perhaps a better choice would have been to replace http:// with https:// in any openstreetmap string after localization?

I'm not trying to be hardnosed about this, I just want to do what is the least burden to localizers.
Patching every localization repo is painful and can confuse localizers which would get a new head when they push their latest translation of other texts. What I find feasible:
Add a code which checks the url format and updates it: https://dxr.mozilla.org/comm-central/rev/bd1684a179caaf03f218e5ecb5aaec6ac27bb5af/mail/base/modules/mailMigrator.js#99
Flags: needinfo?(aryx.bugmail)
Comment on attachment 8696252 [details] [diff] [review]
patch

This is a string change that we should not land this late. We could still probably accept something in TB 45 if it was one of the mitigation strategies that have been suggested that do not impact localization.
Attachment #8696252 - Flags: approval-comm-beta? → approval-comm-beta-
Not going to block on this.
I have posted to mozilla.dev.l10n so everything should be finished here.
Attachment #8696252 - Flags: approval-comm-beta-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: