-
Notifications
You must be signed in to change notification settings - Fork 197
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
Input events dispatch to top-level frame #1847
base: master
Are you sure you want to change the base?
Conversation
@whimboo WDYT? |
The following two things seem to be still missing:
|
Yes, absolutely. By changing the way where we dispatch the events the coordinates should be exactly the same. Not taking care of offsets would cause quite a lot of regressions for those users who make use of actions a lot. Therefore see: web-platform-tests/wpt#48147 Given that Chrome seems to already dispatch actions in the top-level browsing context the referenced tests are failing.
Yes, we would have to do it for all the pointer and wheel input sources. Keyboard actions are more challenging because they could allow users to trigger shortcuts that access restricted browser features. This would require additional checks to determine what actions are permitted and which ones should be blocked. Maybe we should have a follow-up issue for it? |
Aren't all the actions end up in perform implementation-specific action dispatch steps? |
such that trusted events corresponding to the entries in | ||
<var>list of events</var> are dispatched. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively we can "hand-wave" hear like "These steps must be equivalent to user trying to perform the given input device manipulations on context through the top-level browsing context."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should have more wdspec tests (similar to mine for |
@sadym-chromium it looks like each action type dispatches on the context parameter https://w3c.github.io/webdriver/#dfn-dispatch-a-keydown-action
that's right, overlooked that. |
so actually I am not sure, even with this change it is not quite defined what it means that the actions are dispatched to the top-level browsing context. For example, in Chrome that is the case but still would not give you access to browser shortcuts. |
d1e3343
to
88e7b6b
Compare
Yes you are right. I mixed it up with the native event dispatching that I'm working on as well right now. In those cases we would have that particular issue but not when dispatching it in the content process of the top-level browsing context. |
must be equivalent to performing the given input device manipulations | ||
on <var>context</var>, such that trusted events corresponding to the | ||
entries in <var>list of events</var>are dispatched. | ||
dispatch steps</dfn> on a <var>context</var>, and a <var>list of events</var> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep browsing context indicating the context var type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I missed it.
@@ -8392,7 +8438,7 @@ <h3>Processing actions</h3> | |||
</dd> | |||
</dl> | |||
|
|||
<li><p>Return (<var>x</var>, <var>y</var>) | |||
<li><p>Return (<var>x</var> + <var>parentOffsetLeft</var>, <var>y</var> + <var>parentOffsetTop</var>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the offset be applied to all origins or only the viewport? I think the origin pointer might already be in the top level context coordinates and not sure if the element origin does some adjustments already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect the origin "pointer" to be relative to "context" param of "PerformActionsParameters" of the command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That does not seem to follow from the spec text since input sources are per top-level browsing context.
</li> | ||
<li> | ||
Let <var>navigable</var> be <var>context</var>'s <a>active document</a>'s | ||
[=navigable/parent=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[=navigable/parent=] is a property of a navigable and not the document. Did you mean to get the current navigable instead of a parent (e.g., https://html.spec.whatwg.org/#node-navigable)?
Co-authored-by: Alex Rudenko <[email protected]>
Co-authored-by: Alex Rudenko <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that in principle this is the model we need to change to in order to work with modern browser designs.
However I think the change as written is insufficient, and makes the spec contradictory. In each action we still claim we're dispatching events to |context|
which, afaict after these changes, is not the parent context but the actual child context. The spec is unfortunately rather badly written here, so the text you've modified is apparently normative, but exists in what is essentially an informative section describing the overall model.
I also suggest (perhaps as a followup) that we make it possible for specific actions to provide the context that is used for calculating origins (which must in all cases be an ancestor of the top-level command context). That makes it much easier to construct action chains that interact with both the top level document and iframes.
As discussed in w3c/webdriver-bidi#795, the actions should be dispatched from the top-level browsing context.
Preview | Diff