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

Status: VERIFIED FIXED

(Firefox :: File Handling, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

+++ 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.

Comment 1

[Raymond Lee [:raymondlee]]

Tanvi: could you help us with that please?

Comment 2

[Tanvi Vyas[:tanvi]]

Is this for when a user clicks on a mailto: link and their default mail provider is yahoo?

Comment 3

[Tanvi Vyas[:tanvi]]

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

Comment 4

[Tanvi Vyas[:tanvi]]

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.

Comment 5

[Raymond Lee [:raymondlee]]

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

https://compose.mail.yahoo.com/?To=[e-mail] works fine.

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

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Raymond Lee [:raymondlee] from comment #5)
> (In reply to Tanvi Vyas [:tanvi] from comment #4)
> 
> https://compose.mail.yahoo.com/?To=[e-mail] 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.

Comment 7

[Raymond Lee [:raymondlee]]

Attachment: Part 1: Update mail links to https

Comment 8

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 9

[Tanvi Vyas[:tanvi]]

(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.

Comment 10

[Raymond Lee [:raymondlee]]

Attachment: Part 1: Update mail links to https

Comment 11

[Raymond Lee [:raymondlee]]

Attachment: Part 1. Patch for check-in for mail links

Comment 12

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I think we should close this and file a new bug for updating or removing the Yahoo RSS integration.

Comment 13

[Tanvi Vyas[:tanvi]]

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.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d0855b50f559

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d0855b50f559

Comment 16

[Tanvi Vyas[:tanvi]]

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.

Comment 17

[Axel Hecht [:Pike]]

Is there interest to get this ported to localizations?

Comment 18

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(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

[Axel Hecht [:Pike]]

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.

Comment 20

[:Gavin Sharp [email: [e-mail]]]

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?

Comment 21

[Jeff Beatty [:gueroJeff]]

(In reply to :Gavin Sharp (use [e-mail] 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

Comment 22

[Jeff Beatty [:gueroJeff]]

(In reply to :Gavin Sharp (use [e-mail] 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

Comment 23

[Raymond Lee [:raymondlee]]

(In reply to jbeatty from comment #22)
> (In reply to :Gavin Sharp (use [e-mail] 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

[u60234]

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

[u60234]

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.

Comment 26

[Raymond Lee [:raymondlee]]

(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?

Comment 27

[Raymond Lee [:raymondlee]]

The patch for Bug 840699 would update the gecko.handlerService.defaultHandlersVersion and the new Yahoo https handler would be added.

Comment 28

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Raymond, is there anything we can do to finish this off soon?

Comment 29

[Raymond Lee [:raymondlee]]

Attachment: 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.

Comment 30

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 31

[Mike Connor [:mconnor]]

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.

Comment 32

[Matthew N. [:MattN]]

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.

Comment 33

[Matthew N. [:MattN]]

Raymond, is the needinfo for Gavin needed still?

Comment 34

[Ryan VanderMeulen [:RyanVM]]

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

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c00e3be62b88

Comment 36

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 37

[Camelia Badau [:cbadau], Desktop Test Engineering]

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)?

Comment 38

[Francesco Lodolo [:flod]]

(In reply to Camelia Badau, QA [:cbadau] from comment #37)
> Was a bug filled for update other locale files (comment #29)?
Bug 994248