Closed Bug 843877 Opened 11 years ago Closed 11 years ago

Remove facebook.com UA override

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:tef+, b2g18+ verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 verified)

VERIFIED FIXED
blocking-b2g tef+
Tracking Status
b2g18 + verified
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- verified

People

(Reporter: Harald, Assigned: Harald)

References

Details

(Whiteboard: [Apps Watch List])

Attachments

(1 file)

Facebook recognized the B2G UA correctly in tests. Facebook confirmed from their side.

The override causes issue, as Facebook shows "Get our Android App" notifications which make no sense on Firefox OS.

This is part of the demos at MWC and is currently blocking Marketplace review.
Hold up a second - this most definitely does not work for me. I'm getting a WAP site with the FF OS UA.
tracking-b2g18: ? → ---
Blocks: 759986
(In reply to Harald Kirschner from comment #0)
> This is part of the demos at MWC and is currently blocking Marketplace
> review.

Can you please clarify how having a UA override in place is blocking the Marketplace review?

(In reply to Jason Smith [:jsmith] from comment #1)
> Hold up a second - this most definitely does not work for me. I'm getting a
> WAP site with the FF OS UA.

Harald, my test results concur with Jason's. I sent you the same information that Jason provided in comment 1 at 2:39pm PT today in response to your e-mail.
(In reply to Lawrence Mandel [:lmandel] from comment #2)
> (In reply to Harald Kirschner from comment #0)
> > This is part of the demos at MWC and is currently blocking Marketplace
> > review.
> 
> Can you please clarify how having a UA override in place is blocking the
> Marketplace review?

One of 2 required fixed for FB to remove Get-Android-App links that popped up in various places, because we send the UA with Android.


> (In reply to Jason Smith [:jsmith] from comment #1)
> > Hold up a second - this most definitely does not work for me. I'm getting a
> > WAP site with the FF OS UA.
> 
> Harald, my test results concur with Jason's. I sent you the same information
> that Jason provided in comment 1 at 2:39pm PT today in response to your
> e-mail.

They confirmed that its now fixed on their site. I double-checked Jason and Aaron, they could confirm (after finding that the tool they used for mobile UA spoofing has an outdated FxOS UA). I confirmed with FB team that they check for version 14+ (the version that they found in the wiki at time of implementation).
Assignee: nobody → hkirschner
(In reply to Harald Kirschner from comment #3)

> They confirmed that its now fixed on their site. I double-checked Jason and
> Aaron, they could confirm (after finding that the tool they used for mobile
> UA spoofing has an outdated FxOS UA). I confirmed with FB team that they
> check for version 14+ (the version that they found in the wiki at time of
> implementation).

That would be awesome. You got from them more than us. Once this lands I will check for possible regressions in Gaia FB integration. (with the wrong UA the profile page is shown very poorly)
(In reply to Jose M. Cantera from comment #4)
> (In reply to Harald Kirschner from comment #3)
> 
> > They confirmed that its now fixed on their site. I double-checked Jason and
> > Aaron, they could confirm (after finding that the tool they used for mobile
> > UA spoofing has an outdated FxOS UA). I confirmed with FB team that they
> > check for version 14+ (the version that they found in the wiki at time of
> > implementation).
> 
> That would be awesome. You got from them more than us. Once this lands I
> will check for possible regressions in Gaia FB integration. (with the wrong
> UA the profile page is shown very poorly)

Shouldn't we do some testing *before* we land turning off the UA override on FB integration to make sure we don't have regressions? You can easily test this by pulling out the UA override pref in a custom build.
Attachment #718474 - Flags: review?(fabrice)
We need to make a judgment call here to take this for v1.01 or not on risk vs. reward. Noming as such.
blocking-b2g: --- → tef?
Attachment #718474 - Attachment is patch: true
Severity: blocker → normal
Whiteboard: [Apps Watch List]
Attachment #718474 - Flags: review?(fabrice) → review+
Jason's right, this would need testing prior to landing.  If the current UA override is working we don't see any reason to yank it out at the last minute before shipping v1.0.1 and we can take the time to do this right for v1.1
blocking-b2g: tef? → -
tracking-b2g18: --- → ?
Keywords: qawanted
QA Contact: jsmith
The benefit in this case is that taking the patch will result in Facebook serving a touch site with NO Android advertising. I think this is a prereq for inclusion of Facebook in the Marketplace as well. (cc Harald and Ron for comment.) As Mozilla BD is engaged with Facebook, I expect that any fallout can be dealt with by Facebook before the launch.
hi there,

I tested this by adding to my custom-prefs.js :

user_pref("general.useragent.override.facebook.com",
          "Mozilla/5.0 (Mobile; rv:18.0) Gecko/18.0 Firefox/18.0");

and all the FB integration staff worked perfectly as expected and we got rid of the Android banners in the login page. great! So I think it is safe to land this (from FB integration perspective). 

thanks
(In reply to Jose M. Cantera from comment #10)
> I tested this by adding to my custom-prefs.js :
> 
> user_pref("general.useragent.override.facebook.com",
>           "Mozilla/5.0 (Mobile; rv:18.0) Gecko/18.0 Firefox/18.0");
> 

Thanks for testing!

Adding this override is equivalent to removing the override altogether. In the future, you can simply remove the user_pref line from user.js.
Blocks: 827635
(In reply to Lawrence Mandel [:lmandel] from comment #9)
> The benefit in this case is that taking the patch will result in Facebook
> serving a touch site with NO Android advertising. I think this is a prereq
> for inclusion of Facebook in the Marketplace as well. (cc Harald and Ron for
> comment.) As Mozilla BD is engaged with Facebook, I expect that any fallout
> can be dealt with by Facebook before the launch.

There is a pretty strict deal with Facebook as Harald mentioned to me here. If we don't do this, it will indeed make more an awkward conversation with them.

Looks like comment 10 shows this appears to be okay. Let's try the approach of landing this on master, giving a risk analysis on the bug for approval, and noming to see what uplift we'll allow here then.
Removing qawanted per testing on comment 10
Keywords: qawanted
Keywords: checkin-needed
Comment on attachment 718474 [details] [diff] [review]
Remove UA override for Facebook

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

Facebook wasn't recognizing our user agent, so we were severed WAP content on FF OS. They now recognize the user agent, so we are removing the UA override.

User impact if declined: 

Users shall be exposed to Android Google Play advertisements when using Facebook as an app or off the mobile website. This makes no sense in the context of FF OS, as we are not associated with Google Play.

Testing completed: 

Site compatibility testing was done to verify we get the right content now with the FF OS user agent. There was also regression testing stated above to make sure our Facebook integration with the contacts app didn't regress.

Risk to taking this patch (and alternatives if risky):

Low risk - this is modifying the user agent prefs list, so code-wise, the complexity is almost nothing. However, there is slight risks in case this regresses a Facebook integration flow for the Contacts app if there are flows on Facebook's mobile site that somehow didn't fully adapt to the FF OS user agent. Although the testing above reveals that we should be okay.

String or UUID changes made by this patch:

None
Attachment #718474 - Flags: approval-gaia-v1?(21)
Attachment #718474 - Flags: approval-gaia-v1?(21) → approval-gaia-v1+
master: f8b39ac058ce0a73006805df81455d101d971901
v1-train: 1625de716fc75e808436e98cba2c8ff3b1b56153
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
This has been seen during certification.
Device Ikura, Buildid "20130321070205"
gecko commit: b5183c99228bdc5be33340e359efd1b4f0859e92 
gaia commit: 577d13088ebdbd353d13910d3317e713a140415b

Adding dep. and nominating as tef?
blocking-b2g: - → tef?
(In reply to Isabel Rios [:isabel_rios] from comment #16)
> This has been seen during certification.

What exactly has been seen? Do you see Android content? Desktop content?

> Adding dep. and nominating as tef?

This bug has already been resolved and has landed on the b2g18 branch. Can you clarify what you expect to happen via the tef? nom?
(In reply to Lawrence Mandel [:lmandel] from comment #17)
> (In reply to Isabel Rios [:isabel_rios] from comment #16)
> > This has been seen during certification.
> 
> What exactly has been seen? Do you see Android content? Desktop content?
> 
> > Adding dep. and nominating as tef?
> 
> This bug has already been resolved and has landed on the b2g18 branch. Can
> you clarify what you expect to happen via the tef? nom?

It is strange but we are seeing the 'Download Android App' banner in the tests we are making on Inari devices, thus may be there are some patches missing in the OEM's build.
We probably should just tef+ this and land it on 1.01. If not, I expect someone will modify this through a build customization to make this happen anyways, so let's just do this the clean way to not avoid injecting the overhead I think will happen if we don't uplift this.
blocking-b2g: tef? → tef+
James - Could you uplift this to v1.01?
Flags: needinfo?(jlal)
Flags: needinfo?(jhford)
v1.0.1 11bcf1b
Flags: needinfo?(jhford)
Flags: needinfo?(jlal)
"Get our Android App" notification no longer appear when Facebook site is opened in Mozilla OS. Tested on Inari and Leo devices.

Environmental  Variables:
Inari Build ID: 20130503070205
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/3f3489356bbc
Gaia: 3e232bce289c9e156d92553e752616cba284bc8f

Environmental  Variables:
Leo Build ID: 20130503070204
Kernel Date: Apr 25
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8becaf2a0bc7
Gaia: b0aca0dd1e2955e11190ede725e1fb9ee596438b
Status: RESOLVED → VERIFIED
I just downloaded the FB app from Marketplace on ZTE and it still shows "Get Facebook for Android.  Runing ZTE Open with OPEN_FFOS_v1.0.0B02_TME
(In reply to Simon Callan, Telefonica from comment #25)
> I just downloaded the FB app from Marketplace on ZTE and it still shows "Get
> Facebook for Android.  Runing ZTE Open with OPEN_FFOS_v1.0.0B02_TME

On what build ID? You need to running a build later than 5/1/2013.
The build is 2 weeks old - so I'll get the latest and recheck
this bug was reported again during the Latam Certification

Built ID 20130823171628
Device: Ikura
QC RIL version: "ro.build.firmware_revision=V1.01.00.01.019.180"
gaia commit: 4916f82 Merge branch 'zte/ics_strawberry_cdr' of ssh://10.67.16.41:29418/quic/lf/releases/gaia into ics_strawberry_cdr
gecko commit: a1c2bb0 ZRL modify ACCEPT Http head for MMS
This bug has been fixed and verified several months ago. Please open a new bug if you have a new issue.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: