When using touch events, clicking an element with an active state defined in CSS does not cause a transition into that state. I'm working on a patch for this.
Created attachment 56884 [details] Proposed patch and test.
Attachment 56884 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/page/EventHandler.cpp:2771: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 56886 [details] Patch and layout test changes. Correct style on switch statement.
ping .... thoughts anyone? :)
Hi Ben and sorry for late reply. I was thinking about the definition of the :active pseudo-class: "•The :active pseudo-class applies while an element is being activated by the user. For example, between the times the user presses the mouse button and releases it. On systems with more than one mouse button, :active applies only to the primary or primary activation button (typically the "left" mouse button), and any aliases thereof." (http://www.w3.org/TR/css3-selectors/#the-user-action-pseudo-classes-hover-act) Should we only do transition to the active state from left mouse button events since those are the ones that actually activate e.g. a link? And then mapping touch events to mouse events on system/whatever level makes the links work and also enable the active state transitions (since eventhandler thinks that left mouse button was used). This is just a thought, what do you think?
(In reply to comment #5) Hey Kim, Thanks for the comments. We don't convert the touches on android to mouse events until the touch sequence is complete - i.e. the finger is released again, by which time the active class wouldn't apply anymore. Although they are similar, I don't think mouse and touch events are the same enough to do conversion sooner than this. We don't know the intent of the user's touch until later - were they just meaning to pan the page or perform a long press for the context menu for example (in which case we wouldn't want to trigger links/send mouse events to webcore ... or set the active state). There's a good example of using active on divs with touch handlers at this page: http://xdev.org/anim/browser/ the divs should turn blue while you are touching them. The motivation behind trying to come up with a fix for this is to bring WebKit touch support in line with that of the iPhone :) Cheers, Ben
Ah.. Right, that's a good point, I see now :) And it is common not to send the left mouse button press immediately.. This is fine on my behalf.
Thanks for taking a look Kim. I suspect that I'll need to rebase this patch now but rather than just uploading a new rebased one I could also make any changes that reviewers suggest at the same time. Any thoughts guys? :) Thanks, Ben
Created attachment 62058 [details] Rebased patch / layout test.
Attachment 62058 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/page/EventHandler.cpp:2857: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 62063 [details] Rebased patch / layout test, fixed style error.
Hmm, strange that the Mac EWS isn't happy - built fine for me locally. I'm trying a clean build locally to try and see what the problem is.
Hmm, built completely fine on my local machine. Perhaps some flakiness in the bot... doesn't seem to provide any extra information other than "fail"...
Comment on attachment 62063 [details] Rebased patch / layout test, fixed style error. Kim's the expert here, but given his previous review, r=me
Comment on attachment 62063 [details] Rebased patch / layout test, fixed style error. I can't repro the Mac build problem, so going to try landing. Will watch the build pages carefully.
Patch landed as r63807; there was a break on Mac (not sure why it didn't repro on my local MacBook build though) and the fix for that break (simple xcodeproj update) landed in r63808.
Using an int for hitType is ugly. This should be a typedef.
Created attachment 62215 [details] Use a typedef as suggested.
Reopening based on SImon's comments.
Comment on attachment 62215 [details] Use a typedef as suggested. You should use the same type in HitTestRequest, like this: class HitTestRequest { public: enum RequestTypeFlags { ReadOnly = 1 << 1, ... }; typedef unsigned HitTestRequestType; HitTestRequest(HitTestRequestType requestType) ... private: HitTestRequestType m_requestType; } etc.
Created attachment 62222 [details] Patch with typedef. I couldn't find anywhere else in WebCore that is using an int/unsigned for this purpose that needed updating. WDYT? Thanks, Ben
Comment on attachment 62222 [details] Patch with typedef. r=me. Thanks for the cleanup!
Comment on attachment 62222 [details] Patch with typedef. Clearing flags on attachment: 62222 Committed r63888: <http://trac.webkit.org/changeset/63888>
This patch claims to have been landed.