Closed Bug 787054 Opened 12 years ago Closed 12 years ago

Make the UA string OS agnostic again, use UserAgentOverrides.jsm to white-list sites that need "Android"

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: fabrice, Assigned: fabrice)

References

Details

Attachments

(1 file, 2 obsolete files)

Bug 782453 provides an easy way to do per-site UA string tweaking. Let's use that to:
- revert the default UA string to not include "Android".
- populate an initial whitelist of known offenders.

I'll do that as soon as I get the list of sites to whitelist.
We're going to need to come up with an initial whitelist of sites and have a policy for maintaining it for the mobile compat initiative. We also need to know what the app review policy is in connection to this whitelist as well.
(In reply to Jason Smith [:jsmith] from comment #1)
> We're going to need to come up with an initial whitelist of sites and have a
> policy for maintaining it for the mobile compat initiative. We also need to
> know what the app review policy is in connection to this whitelist as well.

Actually, I've got an idea on the apps side for this. I'll followup with Lisa on this on an email on how to handle this.
There seems to be a lot of concern for doing this on #openwebapps on the following factors:

1. The whitelist maintainment would be a burden, as the list would be large and not scalable to maintain
2. Policy on updating this whitelist - how is it updated?

Lawrence - Thoughts?
(In reply to Jason Smith [:jsmith] from comment #3)
> There seems to be a lot of concern for doing this on #openwebapps on the
> following factors:
> 
> 1. The whitelist maintainment would be a burden, as the list would be large
> and not scalable to maintain
> 2. Policy on updating this whitelist - how is it updated?
> 
> Lawrence - Thoughts?

One proposal from the discussion being thrown around was doing something like we do with the blocklist.xml for plugin blocklisting, so that we can update the file easily off of a server accessible to multiple parties.
I didn't want to clutter a bug so I started a thread at https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.webapps/37gOObifr0o about this after the discussion in #openwebapps .  Happy to talk about it wherever though.
blocking-basecamp: --- → ?
Removing blocking nomination for now, this needs a much bigger discussion than what we'll be able to achieve in this bug. If we down the road decide we need to do this, we'll block on this bug, but until then we won't worry about it here.
blocking-basecamp: ? → ---
(In reply to Jason Smith [:jsmith] from comment #3)
> 1. The whitelist maintainment would be a burden, as the list would be large
> and not scalable to maintain
> 2. Policy on updating this whitelist - how is it updated?
> 
> Lawrence - Thoughts?

I think we should intentionally keep the whitelist small, restricting it to only those sites that are determined as must haves for the Firefox OS experience. We should actively be working with the sites to get them off of the list.

We should establish criteria for adding a site to the list such as:
- the site is a must have experience for the platform (Google, YouTube, Facebook, Twitter, etc.)
- evangelism efforts have not proved successful or the company will not be able to make the updates by the launch (Orkut may fall into this category)

Sites should be removed from the list when they are shown to work with the default UA.
(In reply to Wil Clouser [:clouserw] from comment #5)
> I didn't want to clutter a bug so I started a thread at
> https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.webapps/
> 37gOObifr0o about this after the discussion in #openwebapps .  Happy to talk
> about it wherever though.

The link to the discussion didn't work for me. Here is another link:
https://groups.google.com/group/mozilla.dev.webapps/browse_thread/thread/dfb80e39b89faf4a#
Blocks: 785647
Depends on: 782453
OS: Gonk → All
Summary: Use site-specific User Agent infrastructure to white-list sites that need "Android" → Make the UA string OS agnostic again, use UserAgentOverrides.jsm to white-list sites that need "Android"
Is somebody working on this? Fabrice?
(I don't think we need a comprehensive list to start here. We can extend it over time, as needed.)
(In reply to Dão Gottwald [:dao] from comment #10)
> Is somebody working on this? Fabrice?

I have a wip patch, but I was waiting for a more official "go" given the level of controversy this has raised.
When can we expect a decision to be made on this?  The app review team is on hold in discussions with bizdev partners until we know exactly what UA they'll have to support.
Lisa: we realise that this issue needs urgent resolution. I think the answer is _probably_ that the UA they can expect is the OS-agnostic one, but that they may not get it from current B2G builds for a little while, while we get the list of exceptions sorted out to a level which makes people happy to switch back.

The OS-agnostic UA is:
Mozilla/5.0 (Mobile; rv:12.0) Gecko/12.0 Firefox/12.0
(obviously with the appropriate Gecko/Firefox version number instead of 12.0).

However, we should be strongly encouraging partners not to sniff for UAs. The fact that people do that, and that their code inevitably gets it wrong, is why we are in the current mess. Exactly what the right thing is to do instead depends on the partner's use case.

Gerv
Thanks Gerv...I totally agree that UA sniffing isn't the preferred method, but that's what some of our partners are doing, so we gotta work with it.

And it's a minimal wrinkle for us that the final solution isn't implemented yet, more important that we know what the target is.  So app review is good with this for now.
Depends on: 790958
Attached patch wip patch (obsolete) — Splinter Review
wip patch, with only a preference to switch the UA for facebook.com
Assignee: nobody → fabrice
Comment on attachment 661117 [details] [diff] [review]
wip patch

>+pref("general.useragent.override.facebook.com", "\(Mobile;*#\(Android; Mobile");

;* in the reg. exp. will make you drop the semicolon after Mobile.

In the replacement string, you're wrongly adding a backslash.
Attached patch patch (obsolete) — Splinter Review
This patch only adds support for facebook.com and youtube.com (without that, youtube sends us rtsp links that we don't support).
Attachment #661117 - Attachment is obsolete: true
Attachment #664561 - Flags: review?(dao)
Comment on attachment 664561 [details] [diff] [review]
patch

>+pref("general.useragent.override.facebook.com", "\(Mobile;*#(Android; Mobile;");

There's not much value in the asterisk after the semicolon.

You said you're covering youtube but I don't see it in this patch...
(In reply to Dão Gottwald [:dao] from comment #19)
> >+pref("general.useragent.override.facebook.com", "\(Mobile;*#(Android; Mobile;");
> 
> There's not much value in the asterisk after the semicolon.

Here's what I'd do:

pref("general.useragent.override.facebook.com", "\(Mobile#(Android; Mobile");
Attached patch patch v2Splinter Review
Sorry, I forgot to qref. I also updated the regexp according to the last comment.
Attachment #664561 - Attachment is obsolete: true
Attachment #664561 - Flags: review?(dao)
Attachment #664622 - Flags: review?(dao)
Comment on attachment 664622 [details] [diff] [review]
patch v2

I'm not a peer here, so this will probably need a second review from somebody else, although I don't know if we even have any official peers for this code yet.
Attachment #664622 - Flags: review?(dao) → review+
Looking at the patch I see that in the two provided cases we are sending the Firefox for Android UA. It looks to me as though this approach is general enough that we can send a completely different UA (say the Android stock or iPhone UA) if we need to. That would look something like,

+pref("general.useragent.override.mysite.com", "\(Mozilla/5.0 (Mobile; rv:14.0) Gecko/14.0 Firefox/14.0#Mozilla/5.0 (iPhone; U; CPU iPhone OS 4_0 like Mac OS X; en-us) AppleWebKit/532.9 (KHTML, like Gecko) Version/4.0.5 Mobile/8A293 Safari/6531.22.7);

Is that correct?
In order to completely replace the UA string for a given site, you'd do something like this:

pref("general.useragent.override.mysite.com", "Mozilla/5.0 (iPhone; U; CPU iPhone OS 4_0 like Mac OS X; en-us) AppleWebKit/532.9 (KHTML, like Gecko) Version/4.0.5 Mobile/8A293 Safari/6531.22.7");
(In reply to Lawrence Mandel [:lmandel] from comment #23)
> Looking at the patch I see that in the two provided cases we are sending the
> Firefox for Android UA. It looks to me as though this approach is general
> enough that we can send a completely different UA (say the Android stock or
> iPhone UA) if we need to. That would look something like,

Sending UA of browser that are Webkit based will not give us the best results. What's wrong with sending Firefox for Android UA? We evangelize sites to send correct content to Firefox on Android right?
We technically can replace the whole UA, and IMHO we should go ahead with the patches to revert the UA string to the more ideal one and add the first few minimal exceptions ASAP.
We should stick to adaptions that are as minimal as possible to get specific site working, and we should prefer evangelizing the sites, only using the overrides as a as-short-term-as-possible workaround, so we can get user satisfaction while working with the site to make things work the right way.
I don't think Lawrence was suggesting we send the iPhone UA, he was just asking if it were possible. And the answer is that it is; there are two possible types of value for the pref, one defining a regexp replacement, the other a total replacement. 

Dao: Am I correct in saying that this patch switches us back to the non-Android UA by default? If so, we should make sure we have B2G team buy-in for that move. Let's not upset them twice :-)

I agree we need a policy for how sites get added to this list. I will produce one for discussion.

Gerv
(In reply to Gervase Markham [:gerv] from comment #27)
> Dao: Am I correct in saying that this patch switches us back to the
> non-Android UA by default? If so, we should make sure we have B2G team
> buy-in for that move.

Yes and yes. See also comment 22.
Gerv, any objection to landing this patch? I'd really like to have it in before we start the dogfooding phase.
I would like to get Andreas or Brendan to confirm that they agree with this change as Andreas was the one who added the "Android" token back to the UA. Gerv and I are following up.
We lack data to make a great decision, but after talking to Fabrice and Faramarz I am in favor.

I don't know which is likelier: dogfooders diligently report sites that look wrong and we add to the whitelist promptly, resolving most such complaints; or dogfooders are confused and unhappy and dogfood usage suffers and we don't add to the whitelist. Probably we'll have a bit of both.

Fabrice was ok with proceeding with the minimal list we have now (Facebook, maybe another site) and growing. Faramarz said he's help get dogfooders briefed on the need to report sites that might need whitelisting.

Of course we may have sites that look better but misbehave (promote Android apps, e.g.). I'm not as concerned about this case. We will need to evangelize such a site no matter what (whether we whitelist them or not). And possibly add some hacks to block Google Play promotions, hopeless web intents, and the like.

/be
Thanks, Brendan. OK, full speed ahead, chaps :-)

Gerv
https://hg.mozilla.org/mozilla-central/rev/14fe82a53d68
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Keywords: verifyme
QA Contact: jsmith
Over the next few days, we'll put together text, blogposts and publicity relating to what people should do when they find B2G site problems, and the process for getting sites added to the UA override list when/if it's found that this is the problem.

Gerv
Confirmed fixed by checking the user agent at the what is my user agent site plus went to youtube.com to verify that one of sites we whitelisted had it's user agent overridden.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: