Summary: | Add TransformActionEvent support | ||
---|---|---|---|
Product: | WebKit | Reporter: | Kim Grönholm <kim.1.gronholm> |
Component: | New Bugs | Assignee: | Kim Grönholm <kim.1.gronholm> |
Status: | RESOLVED INVALID | ||
Severity: | Normal | CC: | abarth, ademar, benm, christian.webkit, dglazkov, eric, laszlo.gombos, ossy, webkit.review.bot |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | PC | ||
OS: | All | ||
Bug Depends on: | 40329, 40123, 40125, 40232 | ||
Bug Blocks: | 39979 | ||
Attachments: |
Description
Kim Grönholm
2010-05-26 12:54:49 PDT
Created attachment 57130 [details]
Adds TransformActionEvent interfaces
Added only the necessary TransformAction event interfaces and not
e.g. any eventhandler hooks that generate and dispatch them.
I will later add another patch that enables generating these events
from multi-touch gestures. This separation is done to make
reviewing easier.
Attachment 57130 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2525049 Attachment 57130 [details] did not build on win: Build output: http://webkit-commit-queue.appspot.com/results/2551003 Created attachment 57194 [details]
Updated patch
Missed some V8 bindings etc. Added updated patch.
Created attachment 57226 [details]
Manual TransformActionEvent test
Uploading a manual test case for TransformAction events.
Created attachment 57229 [details]
Enables generating TransformAction events from touch gestures
This patch enables transformaction events to be generated from touch
gestures by adding gesture handling machinery and eventhandler hooks.
Hi Kim, I think that the second patch should be in a separate bug that is blocked by this one to make reviews easier. What do you think? Thanks, Ben Comment on attachment 57194 [details]
Updated patch
Looks like a good start! I'm not a reviewer but some comments after trying the first patch on Android:
WebCore/dom/Document.idl:313
+ attribute [DontEnum,Conditional=TRANSFORMACTION_EVENTS,EnabledAtRuntime] EventListener ontransformactionstart;
If you want to make this a runtime enabled feature then you also need to add methods to the RuntimeEnabledFeatures class
in WebCore/bindings/generic/RuntimeEnabledFeatures.h|cpp - see how touch does it for an example.
I think as the feature is protected by a compile time guard it is safe to default the runtime flag to true.
And a few whitespace nits that my patch tool complained about:
WebCore/dom/TransformActionEvent.cpp:49
+ int screenY, int clientX, int clientY, bool ctrlKey, bool altKey,
Trailing whitespace
WebCore/dom/TransformActionEvent.cpp:51
+ int translateSpeedX, int translateSpeedY, float scale, float scaleSpeed,
Trailing whitespace
WebCore/dom/TransformActionEvent.h:35
+ static PassRefPtr<TransformActionEvent> create(const AtomicString& type,
Trailing whitespace
WebCore/dom/TransformActionEvent.h:36
+ bool canBubble, bool cancelable, PassRefPtr<AbstractView> view,
Trailing whitespace
WebCore/dom/TransformActionEvent.h:43
+ view, screenX, screenY, pageX, pageY, ctrlKey, altKey, shiftKey,
Trailing whitespace
Would you also mind adding the new files to the Android makefiles? TransformActionEvent.cpp should go in
WebCore/Android.mk and the idl should go into WebCore/Android.derived.v8bindings.mk and
WebCore/Android.derived.jscbindings.mk.
Thanks!
Created attachment 57294 [details]
TransformActionEvent patch #3
Thanks Ben!
I made the changes that you requested and added the new files to the Android makefiles.
Hopefully you have time to test these more and enable support for the event also in the Android port.
At least with QtWebKit on Win7 multi-touch laptop the event and the test page work fine! :)
Created attachment 57509 [details]
TransformActionEvent patch #4
Changed my e-mail address in the Changelogs to my new Nokia e-mail address.
Obsoleted also the touch gestures patch as I'm going to create a separate bug for that.
Comment on attachment 57509 [details]
TransformActionEvent patch #4
r=me
Landing this may require adjusting LayoutTests/platform/chromium/test_expectations.txt
This is work in progress and we must make sure in our communication that the interface is going to change in the future.
Latest patch LGTM as far as Android goes! I'll try to take a look at the other bug/patch tomorrow. Cheers, Ben Created attachment 57744 [details]
TransformActionEvent patch #5
Rebased the patch.
http://trac.webkit.org/changeset/60614 might have broken Qt Linux Release Committed r60614: <http://trac.webkit.org/changeset/60614> Committed r60615: <http://trac.webkit.org/changeset/60615> Committed r60617: <http://trac.webkit.org/changeset/60617> Rolled out by http://trac.webkit.org/changeset/60624 http://trac.webkit.org/changeset/60625 http://trac.webkit.org/changeset/60626 because it made fast/dom/Window/window-postmessage-clone.html fail on Mac bots (Tiger, Leopard, Snow Leopard): http://build.webkit.org/results/Leopard%20Intel%20Release%20%28Tests%29/r60623%20%2815561%29/fast/dom/Window/window-postmessage-clone-pretty-diff.html It is a very-very strange bug, because I can't reopen it as other bugs. I can only choose RESOLVED/UNCONFIRMED/VERIED/CLOSED. :o The failing test is due to a separate bug which this patch revealed. The bug is being tracked here: https://bugs.webkit.org/show_bug.cgi?id=40232 Created attachment 58935 [details]
TransformActionEvent patch #6
Rebased and added the necessary expected results to qt tests.
Attachment 58935 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3282236 Could you Ben take a look at what is wrong? To me it looks like the patch is effectively identical to the previous ones that didn't break the chromium build. Has something changed somewhere? (In reply to comment #23) > Could you Ben take a look at what is wrong? To me it looks like the patch is effectively identical to the previous ones that didn't break the chromium build. Has something changed somewhere? Hmm, looks to me like the EWS bot didn't regenerate the makefile for the derived bindings to add in the new TransformAction.idl... Eric, Dimitri, what do you think? Is it possible to get the EWS to try again and force it to do a clean build? Created attachment 59229 [details] TransformActionEvent patch #7 Modified the patch so that it changes the expected result of LayoutTests/fast/dom/Window/window-postmessage-clone.html so that it doesn't fail due to a separate bug. See: https://bugs.webkit.org/show_bug.cgi?id=40329 Attachment 59229 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3334508 Comment on attachment 59229 [details]
TransformActionEvent patch #7
Postponing the review until we have more progress on the standardization side.
Re-opening as it's not landed. Comment on attachment 57509 [details] TransformActionEvent patch #4 Cleared Simon Hausmann's review+ from obsolete attachment 57509 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 57744 [details] TransformActionEvent patch #5 Cleared Simon Hausmann's review+ from obsolete attachment 57744 [details] so that this bug does not appear in http://webkit.org/pending-commit. This patch was applied to QtWebKit-2.1 but now it has been reverted. Nobody is requiring this anymore, so we'll wait for it be standardized and get into trunk first. This development is not active anymore. |