Use HTTPS instead of HTTP for Yahoo integration (Yahoo Mail and Yahoo RSS)

VERIFIED FIXED in Firefox 31

Status

()

VERIFIED FIXED
6 years ago
3 years ago

People

(Reporter: briansmith, Assigned: raymondlee)

Tracking

(Depends on: 1 bug)

13 Branch
Firefox 31
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox31 verified)

Details

Attachments

(2 attachments, 2 obsolete attachments)

+++ This bug was initially created as a clone of Bug #840699 +++
+++ This bug was initially created as a clone of Bug #771788 +++

browser.contentHandlers.types.1.uri;http://add.my.yahoo.com/rss?url=%s
gecko.handlerService.schemes.mailto.0.uriTemplate;http://compose.mail.yahoo.com/?To=%s

These should use https:// links. However, neither of those domains uses a valid certificate when I change the "http://" to an "https://", so we need new URLs. Maybe Tanvi can help track down who we'd need to contact.
(Assignee)

Comment 1

6 years ago
Tanvi: could you help us with that please?
Flags: needinfo?(tanvi)

Comment 2

6 years ago
Is this for when a user clicks on a mailto: link and their default mail provider is yahoo?
Flags: needinfo?(tanvi)

Comment 3

6 years ago
I've sent an email to a friend at Yahoo to see if they can take a look at the bug.

Comment 4

5 years ago
My friend got back to me and said the https version should work now.  Looks like this the code that needs to be updated to add the "s":

http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser-region/region.properties

http://mxr.mozilla.org/mozilla-central/source/mobile/locales/en-US/chrome/region.properties

The change could also probably use mochitests if test cases don't already exist.
(Assignee)

Comment 5

5 years ago
(In reply to Tanvi Vyas [:tanvi] from comment #4)

https://compose.mail.yahoo.com/?To=raymond@appcoast.com works fine.

However, https://add.my.yahoo.com/rss?url=http://feeds.bbci.co.uk/news/rss.xml displays an invalid security certificate.
(In reply to Raymond Lee [:raymondlee] from comment #5)
> (In reply to Tanvi Vyas [:tanvi] from comment #4)
> 
> https://compose.mail.yahoo.com/?To=raymond@appcoast.com works fine.

Yes, I verified that. Raymond, do you want to create the patch for this part?

> However,
> https://add.my.yahoo.com/rss?url=http://feeds.bbci.co.uk/news/rss.xml
> displays an invalid security certificate.

Yes, I verified this too. We should move this to another bug, so we don't delay the improvement for compose.mail.yahoo.com.
Assignee: nobody → raymond
(Assignee)

Comment 7

5 years ago
Created attachment 750284 [details] [diff] [review]
Part 1: Update mail links to https
Attachment #750284 - Flags: review?(bsmith)
Comment on attachment 750284 [details] [diff] [review]
Part 1: Update mail links to https

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

Looks good to me, except for that trailing space, but I am not a Firefox reviewer, so sr?gavin.

::: browser/locales/en-US/chrome/browser-region/region.properties
@@ +27,5 @@
>  gecko.handlerService.schemes.webcal.0.uriTemplate=http://30boxes.com/external/widget?refer=ff&url=%s
>  
>  # The default set of protocol handlers for mailto:
>  gecko.handlerService.schemes.mailto.0.name=Yahoo! Mail
> +gecko.handlerService.schemes.mailto.0.uriTemplate=https://compose.mail.yahoo.com/?To=%s 

Remove the trailing whitespace.
Attachment #750284 - Flags: superreview?(gavin.sharp)
Attachment #750284 - Flags: review?(bsmith)
Attachment #750284 - Flags: review+

Comment 9

5 years ago
(In reply to Raymond Lee [:raymondlee] from comment #5)
> However,
> https://add.my.yahoo.com/rss?url=http://feeds.bbci.co.uk/news/rss.xml
> displays an invalid security certificate.

I will ask about this.
(Assignee)

Comment 10

5 years ago
Created attachment 750357 [details] [diff] [review]
Part 1: Update mail links to https
Attachment #750284 - Attachment is obsolete: true
Attachment #750284 - Flags: superreview?(gavin.sharp)
Attachment #750357 - Flags: review?(gavin.sharp)
Attachment #750357 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 11

5 years ago
Created attachment 754349 [details] [diff] [review]
Part 1. Patch for check-in for mail links
Attachment #750357 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #754349 - Flags: checkin?
Attachment #754349 - Flags: checkin? → checkin+
I think we should close this and file a new bug for updating or removing the Yahoo RSS integration.

Comment 13

5 years ago
M(In reply to Tanvi Vyas [:tanvi] from comment #9)
> (In reply to Raymond Lee [:raymondlee] from comment #5)
> > However,
> > https://add.my.yahoo.com/rss?url=http://feeds.bbci.co.uk/news/rss.xml
> > displays an invalid security certificate.
> 
> I will ask about this.

They said they just requested a new cert.  Not sure how long it will take to get one and deploy it.
https://hg.mozilla.org/mozilla-central/rev/d0855b50f559
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24

Comment 16

5 years ago
Reopening since this only solves 1 of the 2 issues.  We could also file a separate bug for the second issue, as bsmith suggested, but for now I just want to make sure we don't lose track of this.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 17

5 years ago
Is there interest to get this ported to localizations?
(In reply to Axel Hecht [:Pike] from comment #17)
> Is there interest to get this ported to localizations?

Yes. I'm not exactly sure what you mean, but the overall goal is to change every http:// link embedded in Firefox to https://, in general.

Comment 19

5 years ago
How do you think this is going to happen then? Seriously, we're a localized product, if you're touching localized files and expect the changes to migrate, you can't just sit there silently.

http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=compose.mail.yahoo.com shows the size of the problem.
The snark isn't really necessary. I missed the localization impact, my bad.

Raymond, can you file a bug to patch all of the localizations as well?
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #20)
> The snark isn't really necessary. I missed the localization impact, my bad.
> 
> Raymond, can you file a bug to patch all of the localizations as well?

Raymond, have you already filed this bug? If not, will you please CC me on it? Thanks
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #20)
> The snark isn't really necessary. I missed the localization impact, my bad.
> 
> Raymond, can you file a bug to patch all of the localizations as well?

Raymond, have you already filed this bug? If not, will you please CC me on it? Thanks
(Assignee)

Comment 23

5 years ago
(In reply to jbeatty from comment #22)
> (In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment
> #20)
> > The snark isn't really necessary. I missed the localization impact, my bad.
> > 
> > Raymond, can you file a bug to patch all of the localizations as well?
> 
> Raymond, have you already filed this bug? If not, will you please CC me on
> it? Thanks

I have missed this comment.  Just filed bug 881319.  Thanks!

Comment 24

5 years ago
As the bug is still open, I'm putting it here. I tested this and it does not seems to be working so good.

With a new profile, it works as intended, except that Yahoo seem to have missed to install a necessary intermediate cert, so I get a "sec_error_unknown_issuer" Visiting https://login.yahoo.com beforehand will pick up and store it though.

With an old profile, the new https URL isn't used at all, presumably because the gecko.handlerService.defaultHandlersVersion pref value is not incremented as the comment in region.properties state should be done when changing the URL.

Incrementing the number did made Firefox pick up the new Yahoo URL, but it also kept the old, so I now have two Yahoo Mail handlers listed in Preferences -> Content -> mailto.

Comment 25

5 years ago
Sorry, I meant Preferences/Options -> Applications -> mailto.

Also note that while the new Yahoo https handler is added when incrementing gecko.handlerService.defaultHandlersVersion, Firefox will continue to use the old http URL until the user manually changes the mailto handler to the new Yahoo https handler.
(Assignee)

Comment 26

5 years ago
(In reply to Hasse from comment #24)
> As the bug is still open, I'm putting it here. I tested this and it does not
> seems to be working so good.
> 
> With a new profile, it works as intended, except that Yahoo seem to have
> missed to install a necessary intermediate cert, so I get a
> "sec_error_unknown_issuer" Visiting https://login.yahoo.com beforehand will
> pick up and store it though.
> 
> With an old profile, the new https URL isn't used at all, presumably because
> the gecko.handlerService.defaultHandlersVersion pref value is not
> incremented as the comment in region.properties state should be done when
> changing the URL.
> 
> Incrementing the number did made Firefox pick up the new Yahoo URL, but it
> also kept the old, so I now have two Yahoo Mail handlers listed in
> Preferences -> Content -> mailto.

I missed to update the gecko.handlerService.defaultHandlersVersion pref value last time. I believe we should update that for both region.properties files.

> Sorry, I meant Preferences/Options -> Applications -> mailto.
> 
> Also note that while the new Yahoo https handler is added when incrementing
> gecko.handlerService.defaultHandlersVersion, Firefox will continue to use
> the old http URL until the user manually changes the mailto handler to the
> new Yahoo https handler.

It looks that we only add new entry but removing old one.
http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsHandlerService.js#216(In reply to Hasse from comment #25)

Gavin: what's your suggestion here?
Flags: needinfo?(gavin.sharp)
(Assignee)

Updated

5 years ago
Blocks: 881319
(Assignee)

Comment 27

5 years ago
The patch for Bug 840699 would update the gecko.handlerService.defaultHandlersVersion and the new Yahoo https handler would be added.
(Assignee)

Updated

5 years ago
Depends on: 904350
Raymond, is there anything we can do to finish this off soon?
Flags: needinfo?(raymond)
(Assignee)

Comment 29

5 years ago
Created attachment 8358950 [details] [diff] [review]
Part 2. Patch for check-in for Yahoo RSS

The adding rss url is working with valid cert.

Note: need to file another bug to update other locale files.
Attachment #8358950 - Flags: review?(brian)
Flags: needinfo?(raymond)
Comment on attachment 8358950 [details] [diff] [review]
Part 2. Patch for check-in for Yahoo RSS

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

It seems right to me, but I really know nothing about how stuff in browser/ works or is supposed to work, so I'm not an appropriate reviewer. Please ask a Firefox peer.
Attachment #8358950 - Flags: review?(brian) → feedback+
Comment on attachment 8358950 [details] [diff] [review]
Part 2. Patch for check-in for Yahoo RSS

Stumbled across this bug, seems like a no-brainer.
Attachment #8358950 - Flags: review?(MattN+bmo)
Comment on attachment 8358950 [details] [diff] [review]
Part 2. Patch for check-in for Yahoo RSS

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

The URL works for me with a few different feeds so this seems fine.
Attachment #8358950 - Flags: review?(MattN+bmo) → review+
Raymond, is the needinfo for Gavin needed still?
Flags: needinfo?(raymond)
Keywords: checkin-needed
Target Milestone: Firefox 24 → ---
https://hg.mozilla.org/integration/mozilla-inbound/rev/c00e3be62b88

Thanks for the patch, Raymond! One request, can you please make sure that future patches include commit information when requesting checkin? Makes life much easier for those landing on your behalf. Thanks!
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c00e3be62b88
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
The needinfo was to verify that this patch actually has an effect on existing users without extra steps - I think it doesn't. We might want a separate bug to track that.
Flags: needinfo?(raymond)
Flags: needinfo?(gavin.sharp)
Keywords: verifyme
status-firefox31: --- → fixed
Verified on Windows 7, Ubuntu 13.10 32bit and Mac OSX 10.9 using Firefox 31.0b9 and gecko.handlerService.schemes.mailto.0.uriTemplate : https://compose.mail.yahoo.com/?To=%s works fine. 

Was a bug filled for update other locale files (comment #29)?
Flags: needinfo?(gavin.sharp)
(In reply to Camelia Badau, QA [:cbadau] from comment #37)
> Was a bug filled for update other locale files (comment #29)?
Bug 994248
Flags: needinfo?(gavin.sharp)
Status: RESOLVED → VERIFIED
status-firefox31: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.