1385206 - create UA override for rakuten.co.jp on Firefox Android

Status: REOPENED

(Web Compatibility :: Interventions, enhancement)

Description

[Karl Dubost💡 :karlcow]

When accessing rakuten.co.jp on Firefox Android, we receive a desktop version.

http://www.rakuten.co.jp/

Comment 1

[Karl Dubost💡 :karlcow]

We need to modify https://hg.mozilla.org/mozilla-central/raw-file/tip/mobile/android/app/ua-update.json.in


Current:
http GET http://www.rakuten.co.jp/ 'User-Agent:Mozilla/5.0 (Android 4.4.4; Mobile; rv:54.0) Gecko/54.0 Firefox/54.0' | grep rakutenmobile


Chrome:
http GET http://www.rakuten.co.jp/ 'User-Agent:Mozilla/5.0 (Linux; Android 4.4.4; en-us; Nexus 5 Build/JOP40D) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/42.0.2307.2 Mobile Safari/537.36' | grep rakutenmobile

<li><a href="http://ac.ebis.ne.jp/tr_set.php?argument=ZMhPE4GP&ai=rgp_ichi_160826_01" onclick="this.href='http://rd.rakuten.co.jp/s/?R2=http%3A%2F%2Fac.ebis.ne.jp%2Ftr_set.php%3Fargument%3DZMhPE4GP%26ai%3Drgp_ichi_160826_01&D2=3.22135.139691.881641.32198472&C3=7a47b4eeb1fbb649683dcf082168675c4fcde836'"><img src="https://r.r10s.jp/com/img/home/smart/top/contents/new/20160830_rakutenmobile_200x200.jpg" width="100" height="100" alt="rakutenmobile"></a></li>


Minimal Change:
Just keeping Gecko, but removing Firefox/54.0 in there does the job, instead of a full blown Chrome UA.

http GET http://www.rakuten.co.jp/ 'User-Agent:Mozilla/5.0 (Android 4.4.4; Mobile; rv:54.0) Gecko/54.0' | grep rakutenmobile

<li><a href="http://ac.ebis.ne.jp/tr_set.php?argument=ZMhPE4GP&ai=rgp_ichi_160826_01" onclick="this.href='http://rd.rakuten.co.jp/s/?R2=http%3A%2F%2Fac.ebis.ne.jp%2Ftr_set.php%3Fargument%3DZMhPE4GP%26ai%3Drgp_ichi_160826_01&D2=3.22135.139691.881641.32198472&C3=7a47b4eeb1fbb649683dcf082168675c4fcde836'"><img src="https://r.r10s.jp/com/img/home/smart/top/contents/new/20160830_rakutenmobile_200x200.jpg" width="100" height="100" alt="rakutenmobile"></a></li>

Comment 2

[Karl Dubost💡 :karlcow]

mike, would you prefer the full Chrome UA?

Comment 3

[Karl Dubost💡 :karlcow]

https://reviewboard.mozilla.org/r/162464

Comment 4

[Karl Dubost💡 :karlcow]

Attachment: Bug 1385206 - Adds rakuten.co.jp UA override;

Review commit: https://reviewboard.mozilla.org/r/162466/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162466/

Comment 5

[Mike Taylor [:miketaylr]]

Comment on attachment 8891263 [details]
Bug 1385206 - Adds rakuten.co.jp UA override;

https://reviewboard.mozilla.org/r/162464/#review167812

::: mobile/android/app/ua-update.json.in:18
(Diff revision 1)
>    // bug 1338260, directv.com
>    "directv.com": "Mozilla/5.0 (Linux; Android 6.0.1; SM-G920F Build/MMB29K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.91 Mobile Safari/537.36",
>    // bug 1310951, m.canadiantire.ca
> -  "m.canadiantire.ca": "Mozilla/5.0 (Linux; Android 6.0.1; SM-G920F Build/MMB29K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.91 Mobile Safari/537.36"
> +  "m.canadiantire.ca": "Mozilla/5.0 (Linux; Android 6.0.1; SM-G920F Build/MMB29K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.91 Mobile Safari/537.36",
> +  // bug 1385206, rakuten.co.jp
> +  "rakuten.co.jp": "Firefox.+;;"

Are you trying to only send the `Firefox.+;;` string? 

If you're trying to do what https://bugzilla.mozilla.org/show_bug.cgi?id=1385206#c1 says, shouldn't it be something like:

`"Firefox.+$#"` (That should translate to something like:

```
"Mozilla/5.0 (Android 6.0.1; Mobile; rv:52.0) Gecko/52.0 Firefox/52.0".replace(new RegExp("Firefox.+$", "g"), "")
```

Comment 6

[Mike Taylor [:miketaylr]]

(In reply to Karl Dubost :karlcow from comment #2)
> mike, would you prefer the full Chrome UA?

If removing the Firefox token gets the job done, that sounds good.

Comment 7

[Karl Dubost💡 :karlcow]

Attachment: Bug 1385206 - Adds rakuten.co.jp UA override;

Review commit: https://reviewboard.mozilla.org/r/162856/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162856/

Comment 8

[Karl Dubost💡 :karlcow]

Attachment: Bug 1385206 - Documents the syntax for this file;

Review commit: https://reviewboard.mozilla.org/r/162858/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162858/

Comment 9

[Mike Taylor [:miketaylr]]

Comment on attachment 8891826 [details]
Bug 1385206 - Adds rakuten.co.jp UA override;

https://reviewboard.mozilla.org/r/162856/#review168350

r=me (assuming you built and tested this locally - ping me on irc if you're not sure how to do that)

Can you please squash the first two ua-update.json commits into one?

Comment 10

[Mike Taylor [:miketaylr]]

Comment on attachment 8891827 [details]
Bug 1385206 - Documents the syntax for this file;

https://reviewboard.mozilla.org/r/162858/#review168348

Comment 11

[Karl Dubost💡 :karlcow]

Comment on attachment 8891826 [details]
Bug 1385206 - Adds rakuten.co.jp UA override;

https://reviewboard.mozilla.org/r/162856/#review168350

```
371462[tip]   15627ca26412   2017-07-31 06:53 +0900   kdubost
  Documents the syntax for this file; r?miketaylr

371461:371459   e21966841291   2017-07-31 06:47 +0900   kdubost
  Bug 1385206 - Adds rakuten.co.jp UA override; r?miketaylr

371460   ce178f748a57   2017-07-28 18:03 +0900   kdubost
  Bug 1385206 - Adds rakuten.co.jp UA override; r?miketaylr

371459   5845151f1a2c   2017-07-28 09:22 +0200   cbook
  Backed out changeset 33172a0a496a (bug 1356271) for broken tests after the backout of bug 1351148 on try
```

Now I need to learn how to squash 371460 and 371461 together with hg. :) I started a rebase which seems to have been taken into account `371461:371459` but not sure how to finish it. I will dig documentation.

As it is my first use of reviewboard. I see the menu for Automation. Try a build. I guess I can use that for testing the build. I tested the user agent on desktop responsive mode with the right syntax. and it was working. 

I want to test something else too before we push to mozilla-central.

Comment 12

[Karl Dubost💡 :karlcow]

Comment on attachment 8891826 [details]
Bug 1385206 - Adds rakuten.co.jp UA override;

https://reviewboard.mozilla.org/r/162856/#review168350

> Fixup commits aren’t appropriate for submitting to MozReview (they should be rewritten/folded/squashed before submission).

https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html

I wonder if I should abandon this one and submit a new review. Probably would be simpler than squashing.

Comment 13

[Karl Dubost💡 :karlcow]

Comment on attachment 8891826 [details]
Bug 1385206 - Adds rakuten.co.jp UA override;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162856/diff/1-2/

Comment 14

[Karl Dubost💡 :karlcow]

Comment on attachment 8891827 [details]
Bug 1385206 - Documents the syntax for this file;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162858/diff/1-2/

Comment 15

[Karl Dubost💡 :karlcow]

ok mike,
I check if on the JS side if we would create issues. And it seems not. I think we can move on.
(tested the UA on a desktop with responsive design and the new UA).

Comment 16

[Mike Taylor [:miketaylr]]

Cool, feel free to land (there's an "Autoland" feature in ReviewBoard...)

Comment 17

[Karl Dubost💡 :karlcow]

according to https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/autoland.html you need L3 to autoland. :)

Comment 18

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 2 changesets with 2 changes to 1 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev 3aca42447712 needs "Bug N" or "No bug" in the commit message.
remote: Karl Dubost <[e-mail]>
remote: Documents the syntax for this file; r=miketaylr
remote: 
remote: MozReview-Commit-ID: Kbr3BNIx9Ky
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote

Comment 19

[Karl Dubost💡 :karlcow]

Comment on attachment 8891827 [details]
Bug 1385206 - Documents the syntax for this file;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162858/diff/2-3/

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b365f7994563
Adds rakuten.co.jp UA override; r=miketaylr
https://hg.mozilla.org/integration/autoland/rev/82ed0c46ac34
Documents the syntax for this file; r=miketaylr

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b365f7994563
https://hg.mozilla.org/mozilla-central/rev/82ed0c46ac34

Comment 22

[Sergiu Logigan [:sergiu]]

RDM vs Real Device:
http://prntscr.com/kopcq2

Comment 23

[Thomas Wisniewski [:twisniewski]]

Reopening this one, since we're now tracking the status of webcompat issues with interventions differently.