Summary: | Add touch event handler for LayoutTests/fast/events/touch/frame-hover-update.html. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eunmi Lee <enmi.lee> | ||||||
Component: | Tools / Tests | Assignee: | Eunmi Lee <enmi.lee> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | laszlo.gombos, lucas.de.marchi | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Eunmi Lee
2013-05-08 00:40:51 PDT
Created attachment 201038 [details]
Patch
Comment on attachment 201038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201038&action=review > LayoutTests/ChangeLog:10 > + > + Add touch event handler to set the needTouchEvents to true and send > + touch event to the web process in the WK2. > + You need to explain why, not just how. > LayoutTests/fast/events/touch/frame-hover-update.html:40 > <script> > + // We have to add at least one touch event handler to feed touch event > + // to the web process in the WK2. > + document.addEventListener('touchstart', function() {}, false); > + I dont like WK2 specific comments. This makes it sounds like a work around Comment on attachment 201038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201038&action=review >> LayoutTests/fast/events/touch/frame-hover-update.html:40 >> + > > I dont like WK2 specific comments. This makes it sounds like a work around indeed, web developer is not aware (and should not be aware) of "web process" :) Comment on attachment 201038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201038&action=review >> LayoutTests/ChangeLog:10 >> + > > You need to explain why, not just how. Yes, I will change the comments. >>> LayoutTests/fast/events/touch/frame-hover-update.html:40 >>> + >> >> I dont like WK2 specific comments. This makes it sounds like a work around > > indeed, web developer is not aware (and should not be aware) of "web process" :) Yes, the layout tests are special case of web page. So, I think the node should have touch event handler if that wants to get and test touch events. I will update the patch. Created attachment 201046 [details]
Patch
Update patch for comments.
This change impacts every ports running this tests. Please remove the [EFL] prefix - we only use that for changes that are only impacting the EFL port. Comment on attachment 201046 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201046&action=review > LayoutTests/ChangeLog:12 > + The touch events are sent to the web page only if the page has touch > + event handlers in the WK2. It is because we don't have to send touch > + events if web page does not use that. > + For that reason the layout test case has to have touch event handlers > + if it tests using touch events. Sounds like we are working around a bug. Hover needs to work even if there are no touch event handlers, right? Comment on attachment 201046 [details]
Patch
This is not the right fix. We need to fix the bug rather than working around it.
|