Closed Bug 798694 Opened 13 years ago Closed 13 years ago

Add UA override for yelp.com

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia
Platform:
ARM
Gonk (Firefox OS)
Type:
defect

Tracking

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed, firefox20 fixed)

Status:
VERIFIED FIXED
Milestone:
B2G C2 (20nov-10dec)
Project Flags:
blocking-basecamp +
Tracking Flags:
Tracking Status
firefox18 --- fixed
firefox19 --- fixed
firefox20 --- fixed

People

(Reporter: cjones, Assigned: ochameau)

References

Details

Attachments

(3 files, 1 obsolete file)

Parts 1 and 2: Move UA overrides into gaia
5 bytes, text/plain
fabrice
: review+
Details
part 3: Remove UA overrides from b2g product proper
1.02 KB, patch
fabrice
: review+
Details | Diff | Splinter Review
Pull request 6435 - whitelist yelp.com
166 bytes, text/html
vingtetun
: review+
Details
Yelp worked great with the Android UA. With the new UA, we get served the full desktop site, which is borderline unusable on device.
Lawrence: Can you drive this one home
Assignee: nobody → lmandel
I've written up the policy, as discussed in the forums, here: https://wiki.mozilla.org/Evangelism/UA_Override_List_Policy 1. I've opened tech evangelism bug 799884. 2. I've also filed a bug at http://www.yelp.com/contact 3. The proposed alternative UA is the one containing Android 4. We know this works from historical experience. 5. An override would be for B2G only. Let's give Yelp a couple of days to respond. (Their claimed response time is less than a week.) Gerv
Firefox-OS-specific overrides are defined in /b2g/app/b2g.js.
Component: Networking: HTTP → General
Depends on: 799884
Product: Core → Boot2Gecko
Any word from Yelp, Gerv?
Flags: needinfo?(gerv)
I'm really missing the usable yelp when dogfooding.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #6) > I'm really missing the usable yelp when dogfooding. Is yelp still serving us a desktop site as of today? If so, I think it's time to consider going forward with a UA override, as there's been more then enough time passed after establishing contact with yelp.
Gerv, any update from Yelp?
Nope, no update from Yelp :-( We can go ahead and add the override. Gerv
Flags: needinfo?(gerv)
Attached patch Spoof the b2g UA for yelp (obsolete) — — Splinter Review
Assignee: lmandel → jones.chris.g
Attachment #679212 - Flags: review?(fabrice)
I followed up with a direct contact at Yelp yesterday. Let's give this another week to see if we can get any commitment from them to make the UA detection change on their end.
Comment on attachment 679212 [details] [diff] [review] Spoof the b2g UA for yelp Review of attachment 679212 [details] [diff] [review]: ----------------------------------------------------------------- Chris, can you move this into the gaia build system, in a "ua-overrides.js" files like we already have for custom-prefs.js and payment-prefs.js ?
Attachment #679212 - Flags: review?(fabrice)
I just heard back from Yelp that they are going to "sort this out". Now that I have a direct response, let's give them some additional time to do so before adding the UA override.
(In reply to Lawrence Mandel [:lmandel] from comment #13) > I just heard back from Yelp that they are going to "sort this out". Now that > I have a direct response, let's give them some additional time to do so > before adding the UA override. I'd actually be more in favor of implementing the override first, and backing it out if we end up finding out that they landed support for our user agent. I think it's a more conservative strategy than playing the waiting game, IMO.
I'm OK with that approach if that's how people want to proceed. We can test for the UA fix using Phony on Android.
Thought I should add that according to the UA override policy [1], "The site has proved unresponsive or unwilling to accommodate us (how long we wait for this will depend on factors such as the popularity of the site and the extent of breakage);" I don't think Yelp passes this test at this point. We should really try to avoid adding sites to the domain whitelist, which should be used as a last resort. [1] https://wiki.mozilla.org/Evangelism/UA_Override_List_Policy
(In reply to Lawrence Mandel [:lmandel] from comment #16) > Thought I should add that according to the UA override policy [1], > > "The site has proved unresponsive or unwilling to accommodate us (how long > we wait for this will depend on factors such as the popularity of the site > and the extent of breakage);" > > I don't think Yelp passes this test at this point. Why not? What would need to change before yelp passes this test, in your view?
(In reply to Chris Jones [:cjones] [:warhammer] from comment #17) > Why not? What would need to change before yelp passes this test, in your > view? Well, although this criteria includes an ambiguous amount of time, I received a response from Yelp yesterday indicating that they will "sort this out". I think we should give them more than 1 day to do so. However, it would be good to know when the cutoff point for the whitelist changes will be so that we can make a better informed decision wrt the time that we wait for this type of change.
Attachment #679907 - Attachment description: Part 1: Move UA overrides into gaia → Parts 1 and 2: Move UA overrides into gaia
Attachment #679907 - Flags: review?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #12) > Chris, can you move this into the gaia build system, in a "ua-overrides.js" > files like we already have for custom-prefs.js and payment-prefs.js ? I note this is now happening. Fabrice: is this just a code reorganization, or does it have technical effect? E.g. do the overrides still apply both to B2G-Firefox and B2G-WebRT? Gerv
Assignee: jones.chris.g → nobody
Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known P2 bugs found before or during C1".
Target Milestone: --- → B2G C2 (20nov-10dec)
Assignee: nobody → poirot.alex
Comment on attachment 679909 [details] [diff] [review] part 3: Remove UA overrides from b2g product proper Review of attachment 679909 [details] [diff] [review]: ----------------------------------------------------------------- r=me for this one but the Parts 1 & 2 are not there!
Attachment #679909 - Flags: review?(fabrice) → review+
(In reply to Fabrice Desré [:fabrice] from comment #23) > Comment on attachment 679909 [details] [diff] [review] > part 3: Remove UA overrides from b2g product proper > > Review of attachment 679909 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me for this one but the Parts 1 & 2 are not there! Those are https://github.com/mozilla-b2g/gaia/pull/6304 ! :)
What Vivien said ;).
Attachment #679907 - Flags: review?(fabrice) → review+
Please someone land attachment 679909 [details] [diff] [review], but leave the bug open after doing it. As we still haven't whitelisted yelp.
Keywords: checkin-needed
Whiteboard: [Leave open after merge]
Whiteboard: [Leave open after merge] → [leave open]
Ok so here is the patch in order to finally whitelist yelp.com. Do we agree on whitelisting yelp for now and eventually remove it when they finally get back to us and fix the issue?
Attachment #682029 - Flags: review?(fabrice)
Lawrence asked when the deadline would be for this sort of change; that's useful data to have for this decision and others. Does anyone know the answer to that? Gerv
I did not review because we don't want to land that yet.
Was it intentional that GAIA/custom-prefs.js now needs to be GAIA/build/custom-prefs.js? The use of addprefix looks like it may have been a bit overzealous.
(In reply to Fabien Cazenave [:kaze] from comment #28) > https://github.com/mozilla-b2g/gaia/commit/ > 5f28571c01960f96afce58eea694ee1cdb0a280c > https://github.com/mozilla-b2g/gaia/pull/6304 Thanks for landing!
(In reply to Andrew Sutherland (:asuth) from comment #32) > Was it intentional that GAIA/custom-prefs.js now needs to be > GAIA/build/custom-prefs.js? The use of addprefix looks like it may have > been a bit overzealous. Nope.
Er sorry, only the scaffolding has landed. yelp.com is still affected.
status-firefox18: fixed → ---
status-firefox19: fixed → ---
Benoit will handle the override for yelp.com.
Assignee: poirot.alex → bjacob
hum, Lawrence, you know the patch is ready? It is only matter of landing Pull request 6435 when you give me the agreement to finally whitelist yelp... Should I land it now?
Flags: needinfo?(lmandel)
I missed that. Yes. It's time to land. Can you free Benoit and take care of bug 803741 and bug 804481 at the same time?
Assignee: bjacob → poirot.alex
Flags: needinfo?(lmandel)
Great, then I don't look into the present bug. Still looking into the 2 other ones unless Alexandre has fixes for them too.
Benoit, yes, please go ahead and fix other bugs, I just wanted to avoid unecessary review for this patch. Landed: https://github.com/mozilla-b2g/gaia/commit/a5e8793af608a21128e8caae1fb98c159e0ad3bd
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Component: General → Gaia
Verified Fixed in Unagi Build 20130103070201
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: