1405112 - Record if reporter is on touch screen desktop device

Status: RESOLVED FIXED

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

Description

[Adam Stevenson [:adamopenweb]]

Is it possible to tell if the user is on a touch screen?

If so we should record this into the report, will give more context when trying to reproduce compat issues.

Comment 1

[Mike Taylor [:miketaylr]]

> Is it possible to tell if the user is on a touch screen?

We should be able to read dom.w3c_touch_events.enabled, yeah. Good idea.

http://searchfox.org/mozilla-central/search?q=dom.w3c_touch_events.enabled&path=

Comment 2

[Mike Taylor [:miketaylr]]

To fix this bug, we just need to add another entry similar to the following, for dom.w3c_touch_events.enabled:

https://dxr.mozilla.org/mozilla-central/source/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#21

Comment 3

[JT Mozingo]

Hey there! I think I'd really like to start working on this bug! First one though. Gonna look around for the source code now and see about getting to work on it. Any tips before I get started?

Comment 4

[Mike Taylor [:miketaylr]]

(In reply to JT Mozingo from comment #3)
> Hey there! I think I'd really like to start working on this bug! First one
> though. Gonna look around for the source code now and see about getting to
> work on it. Any tips before I get started?

Hey cool! The very first step will be to get your development environment set up to build Firefox:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build

Once you're able to build Firefox, it's just a question of adding the pref to the file in Comment #2 and attaching a patch for review. Let me know if you get stuck!

Comment 5

[JT Mozingo]

Awesome! Yeah I have gotten all of the source pulled down and made the change in the file, though I would like to be able to build Firefox and have run into some issue. Following the links you have provided on building Firefox with Windows I have installed the C++ stuff through Visual Studio 2017, but I still receive an error when running './mach build'.

This version (19.12.25835) of the MSVC compiler is not supported.
You must install Visual C++ 2017 Update 6 or newer in order to build.

I am going to keep working at getting this installed properly before submitting the patch, but if you have any suggestions I would really appreciate it.

Thanks!

Comment 6

[Mike Taylor [:miketaylr]]

> You must install Visual C++ 2017 Update 6 or newer in order to build.

According to the docs for windows, "Firefox 61+ require Visual Studio 2017 Update 6 or newer to build." Which version of VS 2017 did you build, do you know?

Comment 7

[JT Mozingo]

To be honestly I am not totally certain if this is correct, but in the VS about window it says I have version 15.5.6.

A quick google search yielded this for me: https://blogs.msdn.microsoft.com/whereismysolution/2018/03/07/update-6-of-visual-studio-2017/

It seems like update 6 is in reference to VS version 15.6.2+, this very well could be my issue. I will try and update it today and see if it works out!

Comment 8

[Arshad Kazmi [:arshadkazmi42]]

Is this bug still open? I would like to work on this.

For the fix, I need to add this in the file mentioned in Comment 2


XPCOMUtils.defineLazyPreferenceGetter(details, "dom.w3c_touch_events.enabled", "dom.w3c_touch_events.enabled", true);


Am I on the right path here?

Also which test case should I verify after making this change?

Comment 9

[Mike Taylor [:miketaylr]]

Hello! Yep, this bug is still available. 

And yeah, totally on the right path. 

You can run the webcompat reporter tests locally to make sure nothing weird happened like so, from your repo root:

./mach mochitest -f browser browser/extensions/webcompat-reporter/test/browser/

To verify it's doing what you'd expect (a bit of a hack...), you can change the following pref in about:config:

extensions.webcompat-reporter.newIssueEndpoint

To something like: http://localhost

And then click the Report Site Issue button (in page actions menu in URL bar), and inspect the details param to make sure it showed up as expected

(and example w/o your changes)

http://localhost/?url=https%3A%2F%2Fdxr.mozilla.org%2Fmozilla-central%2Fsource%2Fbrowser%2Fextensions%2Fwebcompat-reporter%2Fcontent%2FWebCompatReporter.jsm%2321&src=desktop-reporter&details=%7B%22gfx.webrender.all%22%3Afalse%2C%22gfx.webrender.blob-images%22%3Atrue%2C%22gfx.webrender.enabled%22%3Afalse%2C%22image.mem.shared%22%3Atrue%2C%22buildID%22%3A%2220180913141435%22%2C%22channel%22%3A%22aurora%22%2C%22mixed+active+content+blocked%22%3Afalse%2C%22mixed+passive+content+blocked%22%3Afalse%2C%22tracking+content+blocked%22%3A%22false%22%7D

Comment 10

[Mike Taylor [:miketaylr]]

(Note, once you click the button, don't be alarmed by the Unable to connect message, you just want to inspect the URL of the new tab)

Comment 11

[Arshad Kazmi [:arshadkazmi42]]

Cool. I will start working on it right away!

Comment 12

[Arshad Kazmi [:arshadkazmi42]]

Done with the changes. Whom should I add as reviewer for this?

Comment 13

[Arshad Kazmi [:arshadkazmi42]]

Attachment: Bug 1405112 - report whether the user has a touch screen when reporting site issues

Comment 14

[Arshad Kazmi [:arshadkazmi42]]

I have checked commit log for this file and saw Gijs has done some review before on this file. So should I add him as reviewer?

Also I can't find you in reviewers search option in phabricator

Comment 15

[Mike Taylor [:miketaylr]]

Yeah, I think Gijs would be great -- he should be able to look at it in the next few days. I'm not a Firefox module peer, so it's possibly that's why I'm not showing up.

Comment 16

[Arshad Kazmi [:arshadkazmi42]]

Ok. thanks.
I have added Gijs as reviewer

Comment 17

[:Gijs (he/him)]

Johann, do you know if we have an JS-accessible way of getting information about whether the machine has a touch screen today?

Comment 18

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #17)
> Johann, do you know if we have an JS-accessible way of getting information
> about whether the machine has a touch screen today?

(As noted on the phab review, the pref doesn't reflect whether the user has a touch screen or not)

Comment 19

[Johann Hofmann [:johannh]]

(In reply to :Gijs (he/him) from comment #17)
> Johann, do you know if we have an JS-accessible way of getting information
> about whether the machine has a touch screen today?

There are two ways that should work, I believe:

```
let gfxInfo = Cc["@mozilla.org/gfx/info;1"].getService(Ci.nsIGfxInfo);
let hasTouch = gfxInfo.getInfo()["ApzTouchInput"] == 1;
```

This is what about:support uses. It has the advantage that it will work in release versions of Firefox, in case you want to uplift. One thing to note is that the "ApzTouchInput" flag is simply not present on non-touch-screen devices.

or

```
let hasTouch = window.matchMedia("(any-pointer:coarse)").matches;
```

Which you should probably prefer since it uses a Web API that, at least theoretically, should be heavily tested and much less likely to break or get removed than an internal support API.

This was _just_ implemented in bug 1035774 and replaces the proprietary -moz-touch-enabled query, which you should really avoid using since it will be removed soon. So you can't ship this in anything before Firefox 64.

Through this query you also have a bunch of other interesting possibilities, such as finding out the preferred input method of the device.

Let me know if you need anything else!

Comment 20

[Arshad Kazmi [:arshadkazmi42]]

So what will be the fix here?

Do I need to use the code from Comment 19 same as mentioned in Comment 8?

Or there is something else which needs to be done for this?

Comment 21

[Mike Taylor [:miketaylr]]

I think we should probably use the first recommendation, after reading a bit about any-pointer:coarse -- that can mean things like a wacom tablet, or tv remote pointer, or different things.

Comment 22

[Arshad Kazmi [:arshadkazmi42]]

So I should use this 

let gfxInfo = Cc["@mozilla.org/gfx/info;1"].getService(Ci.nsIGfxInfo);
let hasTouch = gfxInfo.getInfo()["ApzTouchInput"] == 1;

in the file mentioned in Comment 8?

Comment 23

[Mike Taylor [:miketaylr]]

yeah, we want to put that in WebCompatReporter.jsm (somewhere). It's not a pref, so the solution isn't as simple as before. The way the blockList details property is implemented is probably a good thing to emulate:

https://searchfox.org/mozilla-central/source/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#28
https://searchfox.org/mozilla-central/source/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#108

Comment 24

[Mike Taylor [:miketaylr]]

Actually, forget line 108 -- having a touch screen wont change per-tab :). So just defining a property on details should be fine (and we want it to be enumerable).

Comment 25

[Arshad Kazmi [:arshadkazmi42]]

So It will be added in the same property 'blockDetails', somewhere in the get fuction?

Comment 26

[Mike Taylor [:miketaylr]]

I don't see a blockDetails, but yes, we want to add a new property to the details object. Something like this:

Object.defineProperty(details, "hasTouchScreen", {
  enumerable: true,
  get() {
    // return a boolean to indicate there's a touch screen detected or not
  },
});

Comment 27

[Arshad Kazmi [:arshadkazmi42]]

For testing this I have to follow the same previous steps mentioned in Comment 9?
Or there is someother tests? or I have to write new tests?

Comment 28

[Arshad Kazmi [:arshadkazmi42]]

I did all the test mentioned in Comment 9.
All existing test cases are passed

And manually checking that flag is showing up in the url.

Problem is my laptop is not touch screen, so how can I verify this with a touch screen? Is it possible to do it with some test scripts?

Comment 29

[:Gijs (he/him)]

(In reply to Arshad Kazmi [:arshadkazmi42] from comment #27)
> For testing this I have to follow the same previous steps mentioned in
> Comment 9?
> Or there is someother tests? or I have to write new tests?

You can test by setting the "dom.w3c_touch_events.enabled" preference to `1`.

Comment 30

[Arshad Kazmi [:arshadkazmi42]]

tested with the flag mentioned in Comment 29

works fine.

pushing the code for review now.

Comment 31

[:Gijs (he/him)]

Comment on attachment 9009673 [details]
Bug 1405112 - report whether the user has a touch screen when reporting site issues

:Gijs (out Thu 27 - Sun 30 / 9;  he/him) has approved the revision.

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23bb00c3ef77
report whether the user has a touch screen when reporting site issues r=Gijs

Comment 33

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/23bb00c3ef77