1544359 - Have Report Site Issue detect the Mobify JS library

Status: RESOLVED FIXED

(Web Compatibility :: Tooling & Investigations, enhancement, P1)

Description

[Karl Dubost💡 :karlcow]

The goal is to make the Reporter detects automatically when Mobify is used on a Website.

This is done in the context of Web Compatibility 2019Q2 work
https://github.com/mozilla/webcompat-team-okrs/issues/79

This will require to discover the pattern for Mobify library
On the server side, it will be handled by https://github.com/webcompat/webcompat.com/issues/2845
(see previously Bug 1535479 for FastClick)

Comment 1

[Mike Taylor [:miketaylr]]

Just found https://trends.builtwith.com/websitelist/Mobify.

Testing the first site, https://www.shaw.ca/ (which redirects to https://www.shaw.ca/store/), there is indeed a window.Mobify global:

{
  "Tag": {
    "ua": "Mozilla/5.0 (Linux; Android 7.0; SM-G892A Build/NRD90M; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/67.0.3396.87 Mobile Safari/537.36",
    "options": {
      "enabled": {
        "url": "//cdn.mobify.com/sites/shaw/production/adaptive.min.js"
      },
      "desktop": {
        "capture": false,
        "url": "//a.mobify.com/shaw/a.js"
      }
    },
    "DOMContentLoaded": 1555537815979
  },
  "points": [
    1555537815969
  ],
  "tagVersion": [
    7,
    0
  ],
  "capturing": false,
  "prefix": "x-"
}

window.Mobify should probably be sufficient, or window.Mobify && window.Mobify.Tag, if we have reason to be more paranoid.

Comment 2

[Ksenia Berezina [:ksenia]]

Added the change to the reporter and also modified the JSON key to be extra_labels instead of label according to #1551721 for the POST request.

Tried testing mobify detection on https://www.shaw.ca/ on local instance/staging of webcompat.com, but none of the labels get created (not even the browser and engine). Is there a way to test labels creation locally or on staging ?

Comment 3

[Karl Dubost💡 :karlcow]

So from the issue, I see:

<!-- @browser: Firefox 68.0 -->
<!-- @ua_header: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0 -->
<!-- @reported_with: desktop-reporter -->

and further in the body.

<li>hasMobify: true</li>

no extra_labels was added to the body.
so this is strange. Let me answer on Bug 1551721

Comment 4

[Mike Taylor [:miketaylr]]

I just filed https://github.com/webcompat/webcompat-tests/issues/1839 to test if the staging.webcompat.com/issues/new?label=browser-fenix worked, and it does seem like something is busted here.

<!-- @browser: Firefox 68.0 -->
<!-- @ua_header: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0 -->
<!-- @reported_with:  -->

There should be <!-- @extra_labels: browser-fenix -->, and reported_with should default to web.

https://github.com/webcompat/webcompat.com/blob/81c91db6a1e8e728fd16e78aa7a548b0300baa64/webcompat/form.py#L131

---

A few notes I took on how this should work,

When the browser opens /issues/new, it will end up in the prefill condition,

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/views.py#L256

Inside prepare_form extra_labels are pulled from a label GET param, if it exists.

https://github.com/webcompat/webcompat.com/blob/8741143216933ec9db718e56da4f8824d2be6905/webcompat/helpers.py#L633

However, if there is a POST with json data, that will win, and a form_data[‘extra_labels’] key will exist.

https://github.com/webcompat/webcompat.com/blob/8741143216933ec9db718e56da4f8824d2be6905/webcompat/helpers.py#L639

(So the expected key for the incoming data should be extra_labels, I think.)

That gets saved to the session cookie:

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/views.py#L258

And then added back to the form object when we’re in the create condition:

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/views.py#L273

We then end up in report_issue:

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/issues.py#L22

Which will call build_formdata

If extra_labels exists there, we add it to the metadata_keys dict:

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/form.py#L338-L340

In theory, we should then come up with a list of the extra_labels from get_metadata, and hide those in the issue body:

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/form.py#L220

Then there’s a web hook after the issue is open:
https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/webhooks/helpers.py#L182

It pulls the labels from the issue body:

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/webhooks/helpers.py#L202

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/webhooks/helpers.py#L163-L179

https://github.com/webcompat/webcompat.com/blob/78596f47aee2df83217be949e94928d70273bef1/webcompat/webhooks/helpers.py#L76

(And follow the call stack down to related metadata)

Comment 5

[Mike Taylor [:miketaylr]]

Ksenia, right now labels won't get added locally or on staging, because we have a webhook that sets the label. I'm not sure why the staging webhook isn't working, I think it used to?

All the hooks are failing for some reason, https://github.com/webcompat/webcompat-tests/settings/hooks/10039471

Comment 6

[Ksenia Berezina [:ksenia]]

Attachment: Bug 1544359 - Fixed report site issue extension: added marfeel and mobify support, fixed labels being passed to the server

Fixed report site issue: added marfeel and mobify support, fixed labels being passed to the server

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2f5143408ad
Fixed report site issue extension: added marfeel and mobify support, fixed labels being passed to the server r=twisniewski,Gijs

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/c2f5143408ad

Comment 9

[Karl Dubost💡 :karlcow]

Ryan, Could we uplift this to Android Fennec 68?

Comment 10

[Ryan VanderMeulen [:RyanVM]]

If someone wants to fill out the request, we can consider it.

Comment 11

[Karl Dubost💡 :karlcow]

Comment on attachment 9068406 [details]
Bug 1544359 - Fixed report site issue extension: added marfeel and mobify support, fixed labels being passed to the server

Beta/Release Uplift Approval Request

• User impact if declined: This would make the fix available to current nightly users of Firefox Fennec Nightly (which is stucked in 68). The consequence would that it would help the Webcompat team to have better reporting on webcompat.com
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This doesn't introduce any modifications in the core of the product. It modifies the format of the communications which is handled server side by webcompat.com. It mainly fixes a bug we had in the previous release version which is creating partial reporting compared to what we initially intended.
• String changes made/needed:

Comment 12

[Karl Dubost💡 :karlcow]

Thanks! Reading https://wiki.mozilla.org/Release_Management/Uplift_rules
Making the request and hope I did it right. Feel free to ask any clarifications.
Adding Thomas for double checking.

Comment 13

[Mike Taylor [:miketaylr]]

LGTM, thanks Karl.

Comment 14

[Julien Cristau [:jcristau]]

Comment on attachment 9068406 [details]
Bug 1544359 - Fixed report site issue extension: added marfeel and mobify support, fixed labels being passed to the server

fix for the webcompat reporter extension, which is only enabled in pre-release channels; approved for 68.0b12

Comment 15

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/87f1246a1c3d