1054088 - Default search is http

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect)

Description

[Eric Rescorla (:ekr)]

STR: 
1. Do a search in the bar
2. Look at the google URL
3. It starts with http://

Expected behavior:
Should be https://

Comment 1

[Kevin Grandon :kgrandon]

Hey Eric. Thanks for filing this. I just checked and this issue exists in 2.0 and below. 2.1 uses a new global search bar, and https is the default there. We will be disabling the old browser app in 2.1, and only shipping the new integrated system browser.

Is it ok with you if we marked this fixed since it is fixed in the latest trunk?

Comment 2

[Eric Rescorla (:ekr)]

(In reply to Kevin Grandon :kgrandon from comment #1)
> Hey Eric. Thanks for filing this. I just checked and this issue exists in
> 2.0 and below. 2.1 uses a new global search bar, and https is the default
> there. We will be disabling the old browser app in 2.1, and only shipping
> the new integrated system browser.
> 
> Is it ok with you if we marked this fixed since it is fixed in the latest
> trunk?

Hmmm... I think we should fix it in 2.0 if possible.

Needinfoing clee and gal for their opinion.

Comment 3

[Martin Thomson [:mt:]]

This is pretty serious.  A fix should be trivial, so the only question I'd have is whether it would have any chance of being included by OEMs.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Martin Thomson [:mt] from comment #3)
> This is pretty serious.  A fix should be trivial, so the only question I'd
> have is whether it would have any chance of being included by OEMs.

That's up to CAF, I think.

Comment 5

[Michael Vines [:m1] [:evilmachines]]

(I don't think we have much of an opinion, if this makes it into v2.0 it'll get picked up over here eventually)

Comment 6

[Ben Francis [:benfrancis]]

Don't Panic!

Search engines are configured at build time, this is just the default search engine if no customizations are applied. No code changes are required to make this change on shipping devices.

Please see MDN for how partners should configure search engines when they create a build for their device https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Market_customizations_guide#Browser_bookmarks_and_default_search_engine

Note that homescreen search providers are configured separately https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Market_customizations_guide#Search_provider_customization

Comment 7

[Eric Rescorla (:ekr)]

(In reply to Ben Francis [:benfrancis] from comment #6)
> Don't Panic!
> 
> Search engines are configured at build time, this is just the default search
> engine if no customizations are applied.

Well, it's not exactly awesome to have our own people getting this when
they do their own builds.


> No code changes are required to
> make this change on shipping devices.
> 
> Please see MDN for how partners should configure search engines when they
> create a build for their device
> https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/
> Market_customizations_guide#Browser_bookmarks_and_default_search_engine

I note that some of these examples have http for Google.

So what do partners in fact do?


> Note that homescreen search providers are configured separately
> https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/
> Market_customizations_guide#Search_provider_customization

Comment 8

[Martin Thomson [:mt:]]

(In reply to Eric Rescorla (:ekr) from comment #7)
> I note that some of these examples have http for Google.

That is an easy fix.

Comment 9

[Ben Francis [:benfrancis]]

(In reply to Eric Rescorla (:ekr) from comment #7)
> Well, it's not exactly awesome to have our own people getting this when
> they do their own builds.

Agreed, HTTPS would be better, pull requests are always welcome.

> I note that some of these examples have http for Google.
> 
> So what do partners in fact do?

Change http to https :)

Comment 10

[Eric Rescorla (:ekr)]

(In reply to Ben Francis [:benfrancis] from comment #9)
> (In reply to Eric Rescorla (:ekr) from comment #7)
> > Well, it's not exactly awesome to have our own people getting this when
> > they do their own builds.
> 
> Agreed, HTTPS would be better, pull requests are always welcome.

If this is a real defect (and I think it is) it really should be
fixed without requiring the reporter to provide a PR.


> > I note that some of these examples have http for Google.
> > 
> > So what do partners in fact do?
> 
> Change http to https :)

Hmm... I just had Tony reflash a Flame with a standard base image
and it behaves this way. So it seems likely in some cases we are shipping
people phones like this.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #5)
> (I don't think we have much of an opinion, if this makes it into v2.0 it'll
> get picked up over here eventually)

Ok, I must have misunderstood the thread from a few days ago about CAF cherrypicking fixes from Mozilla for 2.0.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

[Blocking Requested - why for this release]:

We should set the defaults to the desired behavior, rather than rely on partners or whoever is building the image to take action to get the desired behavior.

Comment 13

[Michael Vines [:m1] [:evilmachines]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #11)
>
> Ok, I must have misunderstood the thread from a few days ago about CAF
> cherrypicking fixes from Mozilla for 2.0.

We're doing that for now, yes, until we reach a stable build.  Then we'd look at picking up this fix and anything else once we know where we stand.

Comment 14

[Preeti Raghunath(:Preeti)]

Peter,

Please weigh in here. Should we be blocking on http v/s https Google search?

Comment 15

[Peter Dolanjski [:pdol]]

We should probably block on this even though it isn't a regression.  Yes, partners could fix it in their builds, but they probably won't unless we planned followup and outreach with all of them.  

It's probably something that we should have caught earlier.  Does this apply to the other search engines as well?

Comment 16

[Eric Rescorla (:ekr)]

(In reply to Peter Dolanjski [:pdol] from comment #15)
> We should probably block on this even though it isn't a regression.  Yes,
> partners could fix it in their builds, but they probably won't unless we
> planned followup and outreach with all of them.  
> 
> It's probably something that we should have caught earlier.  Does this apply
> to the other search engines as well?

It appears to apply to bing.

Comment 17

[Ben Francis [:benfrancis]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/23087

Here's a patch!

Comment 18

[Dietrich Ayala (:dietrich)]

Triage discussed and concluded we should make an exception for this patch because risk is low and we should've been doing this by default - secures the Web for all downstream 2.0 consumers who aren't making this configuration change on their own (and upgrades from older branches for those customers). Blocking+.

Comment 19

[Ben Francis [:benfrancis]]

Decision made, no need to bother our CTO :)

Comment 20

[Dale Harvey (:daleharvey)]

Comment on attachment 8475902 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/23087

Looks good

Comment 21

[Martin Thomson [:mt:]]

I'm a little disappointed that you didn't take the opportunity to add an 's' to the youtube and facebook links while you were in there.

Comment 22

[Dietrich Ayala (:dietrich)]

(In reply to Martin Thomson [:mt] from comment #21)
> I'm a little disappointed that you didn't take the opportunity to add an 's'
> to the youtube and facebook links while you were in there.

Good catch Martin! Each needs independent consideration. Can you file the bugs and link them here?

Comment 23

[Martin Thomson [:mt:]]

I just noticed that we send suggestions requests in the clear to Bing.  And we have to, since they have an invalid certificate on that domain.  We should report the certificate error on https://api.bing.com/ to Microsoft.  I've pinged my contacts at Microsoft, but maybe we have better channels for this.

Comment 24

[Ben Francis [:benfrancis]]

(In reply to Martin Thomson [:mt] from comment #23)
> I just noticed that we send suggestions requests in the clear to Bing.  And
> we have to, since they have an invalid certificate on that domain.  We
> should report the certificate error on https://api.bing.com/ to Microsoft. 
> I've pinged my contacts at Microsoft, but maybe we have better channels for
> this.

Yes I noticed this too whilst making the change, thank you for filing the bug.

Note that this is not actually currently used because we don't yet support third party search suggestions from any provider other than EverythingMe, the build time configuration of these values was the first step towards that feature.

Comment 25

[Ben Francis [:benfrancis]]

TBPL was red, rebased and re-pushed to try now the tree is more green.

Comment 26

[Ben Francis [:benfrancis]]

TBPL still red on unrelated unit test failure. Rebasing and pushing again...

Comment 27

[Ben Francis [:benfrancis]]

https://github.com/mozilla-b2g/gaia/commit/47c11fc6c21fdb6970b70adc5f12c338d1750d24

Comment 28

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/06edd086387c2150017b549e6318a61cd7e4fd02

Comment 29

[SandKing]

This issue has been verified successfully on Flame2.0&2.1
Verify video:"verify_1054088.mp4".

Flame2.0 build
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141130000204
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141130.032432
FW-Date         Sun Nov 30 03:24:44 EST 2014
Bootloader      L1TC00011880

Flame2.1 build:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141130001203
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141130.034738
FW-Date         Sun Nov 30 03:47:49 EST 2014
Bootloader      L1TC00011880

Comment 30

[SandKing]

Attachment: Verified video: verify_1054088.MP4