Bug 219024

Created attachment 414318 [details] WIP patch This is WIP, just to check if it builds in other ports and the result of imported/w3c/web-platform-tests/pointerevents/pointerevent_attributes_hoverable_pointers.html

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

Created attachment 414323 [details] WIP patch

Created attachment 414324 [details] WIP patch

Created attachment 414334 [details] WIP patch

I don't understand the build failures on tv and watch platform. I need some help with those.

(In reply to Carlos Garcia Campos from comment #6) > I don't understand the build failures on tv and watch platform. I need some > help with those. It's a classic unified source 'using namespace' conflict. Might be an indirect one (WebCore -> WebKit -> top-level). I've applied your patch and started a build, I'll see if I can find the minimal change required to get things back on track.

(In reply to Tim Horton from comment #7) > (In reply to Carlos Garcia Campos from comment #6) > > I don't understand the build failures on tv and watch platform. I need some > > help with those. > > It's a classic unified source 'using namespace' conflict. Might be an > indirect one (WebCore -> WebKit -> top-level). I've applied your patch and > started a build, I'll see if I can find the minimal change required to get > things back on track. Thanks!

I landed http://trac.webkit.org/changeset/269901/webkit and hit the retry button here (no idea if I waited long enough between doing those things, though... but it should be OK eventually).

It worked, thank you!

Created attachment 414798 [details] Patch

Comment on attachment 414798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414798&action=review > LayoutTests/ChangeLog:10 > + Update expectations of > + imported/w3c/web-platform-tests/pointerevents/pointerevent_attributes_hoverable_pointers.html. The test is now > + timing out, but because tesdriver doesn't correctly handle the iframe element. See bug #219255

<rdar://problem/71747688>

Comment on attachment 414798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414798&action=review r=me, overall the approach looks straightforward. Unfortunately, watchOS build is still failing: In file included from /Volumes/Data/worker/watchOS-7-Build-EWS/build/WebKitBuild/Release-watchos/DerivedSources/WebKit2/unified-sources/UnifiedSource46-mm.mm:8: /Volumes/Data/worker/watchOS-7-Build-EWS/build/Source/WebKit/UIProcess/ios/forms/WKFocusedFormControlView.mm:134:27: error: reference to 'UIEvent' is ambiguous - (BOOL)handleWheelEvent:(UIEvent *)event ^ This looks like another UnifiedSources thing, ugh. I'll try to figure out what the right include is. > Source/WebKit/Shared/WebMouseEvent.h:73 > + const String& pointerType() const { return m_pointerType; } It's a little weird to me that the pointerType is a string that we pass around. Is there a reason to not use an enum?

(In reply to Brian Burg from comment #14) > Comment on attachment 414798 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=414798&action=review > > r=me, overall the approach looks straightforward. > > Unfortunately, watchOS build is still failing: > > In file included from > /Volumes/Data/worker/watchOS-7-Build-EWS/build/WebKitBuild/Release-watchos/ > DerivedSources/WebKit2/unified-sources/UnifiedSource46-mm.mm:8: > /Volumes/Data/worker/watchOS-7-Build-EWS/build/Source/WebKit/UIProcess/ios/ > forms/WKFocusedFormControlView.mm:134:27: error: reference to 'UIEvent' is > ambiguous > - (BOOL)handleWheelEvent:(UIEvent *)event > ^ > > This looks like another UnifiedSources thing, ugh. I'll try to figure out > what the right include is. I suspect this is happening because we're trying to import `PointerEvent.h` in these UI process-side headers, which also pulls in DOM/bindings-related stuff like WindowProxy, FrameView, Node, etc. Instead, we should probably pull the enum types and declarations we need to reference out into a separate header, and just import that instead of `PointerEvent.h`. > > > Source/WebKit/Shared/WebMouseEvent.h:73 > > + const String& pointerType() const { return m_pointerType; } > > It's a little weird to me that the pointerType is a string that we pass > around. Is there a reason to not use an enum?

Comment on attachment 414798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414798&action=review >>> Source/WebKit/Shared/WebMouseEvent.h:73 >>> + const String& pointerType() const { return m_pointerType; } >> >> It's a little weird to me that the pointerType is a string that we pass around. Is there a reason to not use an enum? > > I guess it's because pointerType is defined in the spec as a DOMString. The spec says it can be mouse, pen and touch, but also other devices that must be prefixed. I guess we could use an enum internally indeed, but it's probably out fo the scope of this bug.

Created attachment 415738 [details] Patch for landing

EWS is green this time!

Committed r270582: <https://trac.webkit.org/changeset/270582> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415738 [details].