Bug 115777 – Add touch event handler for LayoutTests/fast/events/touch/frame-hover-update.html.

• Component: Tools / Tests
• Status: NEW

[Eunmi Lee]

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.

[Eunmi Lee]

Created attachment 201038 [details]
Patch

[Kenneth Rohde Christiansen]

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

[Mikhail Pozdnyakov]

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" :)

[Eunmi Lee]

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.

[Eunmi Lee]

Created attachment 201046 [details]
Patch

Update patch for comments.

[Laszlo Gombos]

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.

[Darin Adler]

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?

[Darin Adler]

Comment on attachment 201046 [details]
Patch

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