Mouse Lock: MouseEvent IDL
Created attachment 114740 [details] Patch
http://dvcs.w3.org/hg/webevents/raw-file/default/mouse-lock.html
Comment on attachment 114740 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114740&action=review please fix the bug title before landing. > Source/WebCore/ChangeLog:3 > + Mouse Lock: MouseEvent IDL This is not IDL, right?
Comment on attachment 114740 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114740&action=review >> Source/WebCore/ChangeLog:3 >> + Mouse Lock: MouseEvent IDL > > This is not IDL, right? The intent of the patch is to stub in the minimal C++ to support the IDL change also in this patch: Source/WebCore/dom/MouseEvent.idl. Still think I should rename?
(In reply to comment #4) > (From update of attachment 114740 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=114740&action=review > > >> Source/WebCore/ChangeLog:3 > >> + Mouse Lock: MouseEvent IDL > > > > This is not IDL, right? > > The intent of the patch is to stub in the minimal C++ to support the IDL change also in this patch: Source/WebCore/dom/MouseEvent.idl. > > Still think I should rename? Doh. Sorry, I misread.
Why do you keep posting patches for Mouse Lock? You did not get support for adding Mouse Lock it its current form on webkit-dev or elsewhere, as far as I can tell. The IDL changes in particular are wrong. One cannot add optional arguments in the middle of a function prototype without breaking existing callers.
Comment on attachment 114740 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114740&action=review > Source/WebCore/dom/MouseEvent.idl:51 > +#if defined(ENABLE_MOUSE_LOCK_API) && ENABLE_MOUSE_LOCK_API > + in [Optional=CallWithDefaultValue] long movementX, > + in [Optional=CallWithDefaultValue] long movementY, > +#endif Alexey is right -- this is wrong. Can we remove this for now?
Created attachment 114772 [details] Patch
Committed r100024: <http://trac.webkit.org/changeset/100024>
Reverted r100024 for reason: Broke chromium build Committed r100027: <http://trac.webkit.org/changeset/100027>
Created attachment 114859 [details] Patch
Committed r100093: <http://trac.webkit.org/changeset/100093>
Comment on attachment 114859 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114859&action=review > Source/WebCore/ChangeLog:8 > + Tests for movementX/Y pending mock mouse lock test infrastructure. Frowns. > Source/WebCore/dom/MouseEvent.idl:31 > +#if defined(ENABLE_MOUSE_LOCK_API) && ENABLE_MOUSE_LOCK_API > + readonly attribute long movementX; > + readonly attribute long movementY; > +#endif Can we use [Conditional=MOUSE_LOCK_API] ? Also, why ENABLE(MOUSE_LOCK_API) and not just ENABLE(MOUSE_LOCK) ? Almost everything in ENABLE is an API. :)
(In reply to comment #13) > (From update of attachment 114859 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=114859&action=review > > > Source/WebCore/ChangeLog:8 > > + Tests for movementX/Y pending mock mouse lock test infrastructure. > > Frowns. > > > Source/WebCore/dom/MouseEvent.idl:31 > > +#if defined(ENABLE_MOUSE_LOCK_API) && ENABLE_MOUSE_LOCK_API > > + readonly attribute long movementX; > > + readonly attribute long movementY; > > +#endif > > Can we use [Conditional=MOUSE_LOCK_API] ? > > Also, why ENABLE(MOUSE_LOCK_API) and not just ENABLE(MOUSE_LOCK) ? Almost everything in ENABLE is an API. :) Sure, will follow up with a polish patch.
> Sure, will follow up with a polish patch. Thanks!