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)
Thunderbird
Address Book
Tracking
(thunderbird45- affected, thunderbird46 fixed, thunderbird47 fixed)
RESOLVED
FIXED
Thunderbird 47.0
People
(Reporter: aryx, Assigned: aceman)
Details
Attachments
(1 file)
3.28 KB,
patch
|
mkmelin
:
review+
philip.chee
:
review+
jorgk-bmo
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
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).
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 4•8 years ago
|
||
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+
Updated•8 years ago
|
Attachment #8696252 -
Flags: review?(iann_bugzilla) → review+
Thanks.
Severity: normal → enhancement
tracking-thunderbird45:
--- → ?
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?
Updated•8 years ago
|
Attachment #8696252 -
Flags: approval-comm-aurora? → approval-comm-aurora+
Updated•8 years ago
|
status-thunderbird45:
--- → affected
status-thunderbird46:
--- → affected
status-thunderbird47:
--- → fixed
Comment 8•8 years ago
|
||
Aurora (TB 46): https://hg.mozilla.org/releases/comm-aurora/rev/2a69237d77fe
Comment 9•8 years ago
|
||
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).
Comment 10•8 years ago
|
||
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.
Assignee | ||
Comment 11•8 years ago
|
||
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)
Comment 12•8 years ago
|
||
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.
Reporter | ||
Comment 13•8 years ago
|
||
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 14•8 years ago
|
||
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-
Comment 15•8 years ago
|
||
Not going to block on this.
Assignee | ||
Comment 16•8 years ago
|
||
I have posted to mozilla.dev.l10n so everything should be finished here.
Comment 17•8 years ago
|
||
https://hg.mozilla.org/releases/l10n/mozilla-aurora/et/rev/ec4c8e8311093c3fbf80802c77c670685a473391 Bug 1225935 - Use https versions of Google maps and OpenStreetMap. rs=Fallen
Updated•5 years ago
|
Attachment #8696252 -
Flags: approval-comm-beta-
You need to log in
before you can comment on or make changes to this bug.
Description
•