Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure "Focused and Visible" addresses OS-level focus #747

Closed
johnpallett opened this issue Jun 29, 2019 · 9 comments
Closed

Ensure "Focused and Visible" addresses OS-level focus #747

johnpallett opened this issue Jun 29, 2019 · 9 comments

Comments

@johnpallett
Copy link
Contributor

Pose data should not be available during an XRSession if the browser itself is out of focus (at the OS-level) otherwise that pose data might be used for input sniffing or gaze tracking of other applications.

For example, in the following scenario:

  • A web page includes an inline XR session, which is in focus within the page
  • The browser itself is visible
  • Another non-browser application has focus at the OS level.

... the definition of 'focused' needs to be reviewed to ensure that in this situation, the XR session is not considered 'in focus'.

Note: The existing definition of 'focused' might already cover this. I'm filing this issue so that we can confirm.

@johnpallett
Copy link
Contributor Author

Note: The answer to this question might affect either the #visibility-and-focus section of the privacy explainer

@NellWaliczek
Copy link
Member

Hey John, can you take point on figuring out the answer to this as soon as possible? We'd like to make sure it's covered in the next Working Draft.

@NellWaliczek NellWaliczek added this to the June 2019 milestone Jul 1, 2019
@johnpallett
Copy link
Contributor Author

johnpallett commented Jul 8, 2019

Still trying to work out whether the HTML spec for Focus addresses this explicitly. A few notes in the meantime:

  1. In practice some browsers (at least Chrome, Firefox, Edge) return hasFocus() as false if the browser application is out of focus. Test page here.

  2. The Focus Introduction calls out the 'Web browser' as a participant in focus determination. That intro is non-normative though.

  3. The Generic Sensors API uses focus as a condition for exposing sensor readings.

@johnpallett
Copy link
Contributor Author

@NellWaliczek it's possible this is an HTML spec issue (I'm glad you brought this up!) Even if the spec does cover this elsewhere, I don't see harm in clarifying within the explainer and possibly the spec.

WDYT? If you agree, one approach would be to add a line, something like:

Note: A primary purpose of the focus requirement is to prevent the use of WebXR sensor data to sniff inputs from other applications or the operating system. When considering focus, the Web browser must additionally have focus within its operating system, and the tab must have focus within the Web browser.

@cwilso cwilso changed the title Ensure "Focused and Visibile" addresses OS-level focus Ensure "Focused and Visible" addresses OS-level focus Jul 10, 2019
@cwilso cwilso modified the milestones: June 2019, July 2019 Jul 15, 2019
@NellWaliczek NellWaliczek modified the milestones: July 2019, August 2019 Jul 31, 2019
@NellWaliczek NellWaliczek assigned toji and unassigned johnpallett Aug 15, 2019
@toji
Copy link
Member

toji commented Oct 28, 2019

Okay, so after a lot of research, toying around, and introspection, here's my impressions of what should happen here:

The primary concern is inline sessions. This is because immersive sessions imply some level of exclusivity to the device sensors, and periods where the immersive session is active but should not be receiving poses (a state which is up the UA/XR platform to determine) should be represented by the XRSession.visibilityState. Even if the underlying window wasn't focused it wouldn't be a huge concern because the session is exclusively capturing the pose data. That said, it's likely easier to keep the focus logic consistent between both inline and immersive for ease of implementation and developer comprehension.

So that leaves us evaluating the security needs for inline, which line up extremely closely with that of generic sensors, which John linked above. Currently we are using nearly identical language to them, and if we were to discover a deficiency in that definition then it's likely that we would want to fix it universally and not just for our spec, so this really does seem like a HTML spec issue if anything is indeed under specified.

It is worth noting that every UA on every OS I've been able to test all DO consider OS focus as part of the document's focused state, so behavioral consistency is not a concern here for me so much as that of documentation consistency. It's worth filing an issue about.

There is one potential tweak we may want to make to the spec for clarity, however. Currently we (and Generic Sensors) link to the "currently focused area" definition, but that doesn't seem to account for nested documents of the same origin. It feels like we want to rely on the behavior used by the document.hasFocus() method, which refers to the "Has Focus Steps" algorithm. This does the same thing but checks through the entire document hierarchy for focus, which seems more appropriate to me.

@toji
Copy link
Member

toji commented Oct 28, 2019

HTML spec issue filed here: whatwg/html#5049

@toji
Copy link
Member

toji commented Nov 4, 2019

Upon further inspection, it looks like the "currently focused area" algorithm is actually recursive in the same way as the "has focus steps" algorithm. (See specifically step 2: "If the designated focused area of the document is a browsing context container with a non-null nested browsing context, then let candidate be the active document of that browsing context container's nested browsing context, and redo this step.")

As a result I think no change is needed to the WebXR spec for this specific issue now that the relevant HTML spec issue has been filed, and am inclined to close it. @NellWaliczek or @Manishearth, any concerns?

@Manishearth
Copy link
Contributor

Seems fine. May be worth adding a note somewhere.

@toji toji modified the milestones: October 2019, November 2019 Nov 18, 2019
@NellWaliczek
Copy link
Member

Makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants