New Plugin: Browser Check #2209
Conversation
🦋 Changeset detectedLatest commit: 4493864 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
One thing that I'm planning to do in this plugin is provide information about the browser through userAgent parsing. I know this is generally frowned upon in favor of feature detection, but I think running scientific experiments is a case where userAgent sniffing is justified. The plugin can do feature detection too. One issue I'm running into with this is that I'd like to write tests in jest, but the library I'm using for parsing the userAgent doesn't know what to do with jsdom's user agent string. It looks like the userAgent can be set manually in the jest environment, but I'm not sure where to put that in our new set up. @bjoluc where can we set config options for the testing environment that are specific to the plugin package? |
Good one, I've been manually doing this a lot in my experiments. I've also often logged the result of vsync-estimate in an initial trial. Worth including something like that as well?
Simply in beforeAll(() => {
jest.spyOn(navigator, "userAgent", "get").mockReturnValue("foo");
});may be a good alternative. |
|
Note to self: add mic check, webcam check (just in terms of whether they can be accessed at all). |
| if (trial.inclusion_function(Object.fromEntries(feature_data))) { | ||
| end_trial(); | ||
| } else { | ||
| this.jsPsych.endExperiment(trial.exclusion_message); |
There was a problem hiding this comment.
On second thought, don't do it this way.
Want to record the exclusion in the data before ending the experiment. Will need to think about how to do that since it doesn't quite mesh with plugin norms.
There was a problem hiding this comment.
Should we add a second parameter (e.g. endExperiment) to finishTrial that ends the experiment after finishing the trial?
There was a problem hiding this comment.
This solution is growing on me. Do you think it's better to adjust finishTrial like this:
finishTrial(data = {}, end_experiment = false, end_message = '') {or better to change endExperiment like this
endExperiment(end_message: string, data = {})Right now endExperiment actually sets a flag on the timeline and invokes finishTrial. Then when the timeline goes to the next trial it checks the flag and the experiment ends. So the second modification is probably easier to make in the current implementation.
There was a problem hiding this comment.
And in case it isn't clear, my intention above is that endExperiment would then be able to optionally save data in the final trial object.
TBH, the current implementation of endExperiment is probably not ideal and should change with some larger overhaul of the timeline code.
There was a problem hiding this comment.
Funny, I was initially writing a list that contained exactly these two items but then decided to drop the second option entirely. But now that you bring it up again, I'm not too sure anymore. I think what made me discard it was that I didn't know that endExperiment() calls finishTrial() internally. The more I think about it now, the more I like the extension to endExperiment(). It is explicit and easy to maintain during future refactoring. Maybe finishTrial will also be replaced with returning from an async trial() function one day, and we'd be lucky to have chosen endExperiment(end_message: string, data = {}) then!
|
@bjoluc I'm not sure exactly why the changesets for this release are bumping all the plugins by a major version. I saw this problem popped up in your other PR recently and you fixed it. What should I do here? |
|
@jodeleeuw That's because changesets updates peer dependencies by default, and we're bumping the jspsych package. There's a flag in the changesets config to disable this, which I added on |
| requestAnimationFrame(start); | ||
| }); | ||
| }, | ||
| webcam: () => { |
There was a problem hiding this comment.
I'm not sure this is the right way to handle checking for a webcam and checking for a microphone. Some sources indicate that enumerateDevices() will provide information about hardware capabilities without requesting permission from the user. Other sources make it sound like permission is needed. When I test locally through file:// or a local server I don't get a permission prompt. However, I don't have easy access to devices without a mic or webcam so I can't confirm that this actually works as intended yet.
There was a problem hiding this comment.
In case it's helpful, here's how I did the mic detection in the audio-response plugin: https://github.com/becky-gilbert/jsPsych/blob/6fd6a536d04e9fd032df00cacca149e432c00de6/plugins/jspsych-image-audio-response.js#L205-L220
We've been using this plugin for a while and haven't had any problems with it. But this plugin always requests mic permission from the user (I think via navigator.mediaDevices.getUserMedia here), and I can't remember if it still requests permission when a mic isn't found.
Let me know if I can help by testing on a device without mic/webcam access.
There was a problem hiding this comment.
I'd imagine there's significant variation between browsers and individual configurations. I think testing on chrome and firefox on desktop and mobile will be sufficient for most cases. Combined they account for most browsers and ime firefox tends towards more restrictions on web resources for security and privacy reasons.
As for testing on devices without mics and webcams, I'd suggest an ubuntu or debian virtual machine and deny it access to host hardware. It should behave like a machine without a mic or webcam.
There was a problem hiding this comment.
I've tested Chome and Firefox. Thanks for the suggestion about using a virtual machine. I'll try to get to that soon.
There was a problem hiding this comment.
I think we also need to test on MacOS/iOS and Safari (2nd most used after Chrome), as these seem more likely to not support certain features (or they do but implement them differently 😫). I have an iPad I can test on, if that would help?
|
@jodeleeuw Is your local |
|
I was just being lazy, but I've updated and merged. Not sure why the |
|
I think this one is basically ready to go. Reviews welcome (@becky-gilbert @bjoluc @chrisbrickhouse or whoever!) |
| | minimum_width | int | 0 | If `allow_window_resize` is `true`, then this is the minimum width of the window (in pixels) that must be met before continuing. | ||
| | window_resize_message | string | see description | The message that will be displayed during the interactive resize when `allow_window_resize` is `true` and the window is too small. If the message contains HTML elements with the special IDs `browser-check-min-width`, `browser-check-min-height`, `browser-check-actual-height`, and/or `browser-check-actual-width`, then the contents of those elements will be dynamically updated to reflect the `minimum_width`, `minimum_height` and measured width and height of the browser. The default message is: `<p>Your browser window is too small to complete this experiment. Please maximize the size of your browser window. If your browser window is already maximized, you will not be able to complete this experiment.</p><p>The minimum window width is <span id="browser-check-min-width"></span> px.</p><p>Your current window width is <span id="browser-check-actual-width"></span> px.</p><p>The minimum window height is <span id="browser-check-min-height"></span> px.</p><p>Your current window height is <span id="browser-check-actual-height"></span> px.</p>`. | ||
| resize_fail_button_text | string | `"I cannot make the window any larger"` | During the interactive resize, a button with this text will be displayed below the `window_resize_message` for the participant to click if the window cannot meet the minimum size needed. When the button is clicked, the experiment will end and `exclusion_message` will be displayed. | ||
| inclusion_function | function | `() => { return true; }` | A function that evaluates to `true` if the browser meets all of the inclusion criteria for the experiment, and `false` otherwise. The first argument to the function will be an object containing key value pairs with the measured features of the browser. The keys will be the same as those listed in `features`. See example below. |
There was a problem hiding this comment.
This is fine for the initial release, but in some later version I would recommend adding parameters which automate common inclusion criteria like mobile, operating system, and device restrictions. Callbacks are great and this opens the door to a lot of customization, but compared to the old interface it adds complexity that might give users some trouble when migrating.
There was a problem hiding this comment.
I can see the case for this. I wanted something that was both simple in terms of parameter bloat and also flexible, so this seemed like the simplest path forward for now. I wouldn't be opposed to adding parameters like allowed_browsers or whatever in a future release.
| requestAnimationFrame(start); | ||
| }); | ||
| }, | ||
| webcam: () => { |
There was a problem hiding this comment.
I'd imagine there's significant variation between browsers and individual configurations. I think testing on chrome and firefox on desktop and mobile will be sufficient for most cases. Combined they account for most browsers and ime firefox tends towards more restrictions on web resources for security and privacy reasons.
As for testing on devices without mics and webcams, I'd suggest an ubuntu or debian virtual machine and deny it access to host hardware. It should behave like a machine without a mic or webcam.
|
Sorry, I should've said that I'm happy to make any of the suggested changes myself - just let me know! |
This is currently a work in progress.
This plugin will record a variety of information about the browser in the data and allow the developer to specify a set of inclusion criteria.