Bug 115777 - Add touch event handler for LayoutTests/fast/events/touch/frame-hover-update.html.
Summary: Add touch event handler for LayoutTests/fast/events/touch/frame-hover-update....
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eunmi Lee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-08 00:40 PDT by Eunmi Lee
Modified: 2016-03-09 09:47 PST (History)
2 users (show)

See Also:


Attachments
Patch (2.44 KB, patch)
2013-05-08 00:56 PDT, Eunmi Lee
no flags Details | Formatted Diff | Diff
Patch (2.62 KB, patch)
2013-05-08 02:13 PDT, Eunmi Lee
darin: review-
darin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eunmi Lee 2013-05-08 00:40:51 PDT
The LayoutTests/fast/events/touch/frame-hover-update.html is failed in the efl-wk2
because ui process does not send any touch events to the web process if there is no touch event handler in the webpage.

So, I'm going to add the fake touch event handler to the LayoutTests/fast/events/touch/frame-hover-update.html.
Comment 1 Eunmi Lee 2013-05-08 00:56:39 PDT
Created attachment 201038 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2013-05-08 01:10:02 PDT
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 3 Mikhail Pozdnyakov 2013-05-08 01:29:59 PDT
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 4 Eunmi Lee 2013-05-08 01:54:31 PDT
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.
Comment 5 Eunmi Lee 2013-05-08 02:13:59 PDT
Created attachment 201046 [details]
Patch

Update patch for comments.
Comment 6 Laszlo Gombos 2013-05-10 16:37:54 PDT
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 7 Darin Adler 2014-07-12 19:12:55 PDT
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 8 Darin Adler 2016-03-09 09:47:25 PST
Comment on attachment 201046 [details]
Patch

This is not the right fix. We need to fix the bug rather than working around it.