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

Implement PointerEvents #16105

Open
ferjm opened this issue Mar 23, 2017 · 11 comments
Open

Implement PointerEvents #16105

ferjm opened this issue Mar 23, 2017 · 11 comments

Comments

@ferjm
Copy link
Member

@ferjm ferjm commented Mar 23, 2017

@dowoncha
Copy link

@dowoncha dowoncha commented Mar 29, 2017

@ferjm I am interested in taking this. Any starting tips?

@ferjm
Copy link
Member Author

@ferjm ferjm commented Mar 29, 2017

Hello @dowoncha! Thank you for your interest on taking this task.

I myself am starting with Servo, so apologies in advance if my tips are not accurate enough. I am CCing @jdm and @mbrubeck who know way more than me about this stuff.

First of all, this work involves touching several pieces of Servo, so in case that you haven't already done it, it would be good to read the Design notes. I saw that you already contributed some code to Servo \o/, so I won't mention any specifics about the contribution process, but feel free to ask any question about it.

I would also recommend reading @jeenalee 's blog post about how to implement a new DOM API for Servo, as it covers some of the work required here. In essence, you will need to create a new components/script/dom/webidls/PointerEvent.webidl file with what's defined in the spec and its corresponding components/script/dom/pointerevent.rs implementation. Having the interface and the implementation stubs in place is a good start.

According to the spec, you'll need to extend MouseEvent. So I think understanding what mouse events do will be very useful to understand what's required for PointerEvents.

@jdm and @mbrubeck can correct me if I am wrong, but I believe for mouse events everything starts in Glutin, which is the cross-platform windowing and input lib used in Servo. It seems to support mouse and touch devices but it may need to be extended with pen/tablet input support. I would recommend leaving this work for later.

Glutin events are handled here by Servo's glutin app, which dispatches the events to the compositor.

The Compositor sends a ConstellationControlMsg::SendEvent message with the Pipeline ID and the details of the mouse event.

This message is handled by the script thread which ends up calling document.handle_mouse_event. There is where the creation and dispatch of the DOM MouseEvent happens.

I hope I gave you enough to start. Please, let me know if you have any specific question. Feel free to ping me on IRC as well (:ferjm there). Thank you again for your interest in taking this work.

Happy hacking!

@dowoncha
Copy link

@dowoncha dowoncha commented Apr 5, 2017

The PointerEvent webidl has certain attributes with type long. I expected those to correspond to an i64 but the generated bindings expect an i32. Which type should I target for long attributes?
EDIT: used i32 for long attributes.

I noticed some dom structures such as MouseEvent use Cell for internal fields while others like Touch does not. I have decided to use Cell for PointerEvent but is there a desired guarantee?

@jdm
Copy link
Member

@jdm jdm commented Apr 5, 2017

The types that the generated bindings provide are correct. long has a weird history in IDL.

@dowoncha
Copy link

@dowoncha dowoncha commented Apr 5, 2017

I am now ready to implement the trait items (e.g. GetOnpointerdown, Setonpointerdown, etc). Cargo currently gives me 3 errors in implementing the same functions in DocumentMethods, HTMLElementMethods, or WindowMethods. Which one should I implement in?

@jdm
Copy link
Member

@jdm jdm commented Apr 5, 2017

They should be part of the global_event_handler macro in components/script/dom/macros.rs.

@dowoncha
Copy link

@dowoncha dowoncha commented Apr 5, 2017

cc @jdm I would like to submit a WIP PR, could you open up a pointerevent branch on servo/servo

@dowoncha
Copy link

@dowoncha dowoncha commented Apr 5, 2017

@highfive: assign me

@highfive
Copy link

@highfive highfive commented Apr 5, 2017

Hey @dowoncha! Thanks for your interest in working on this issue. It's now assigned to you!

@highfive highfive added the C-assigned label Apr 5, 2017
@jdm
Copy link
Member

@jdm jdm commented Apr 5, 2017

Opening up a WIP PR is fine; there's no need to have a branch on servo/servo for that.

@dowoncha
Copy link

@dowoncha dowoncha commented Apr 5, 2017

WIP PR #16273

@jdm jdm removed the C-assigned label May 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.