Bug 175864 – [WPE] Some event handlers not working.

• Status: RESOLVED
• Resolution: FIXED
• Component: WPE WebKit

[Carlos Alberto Lopez Perez]

The following simple test with the onclick event handler is not working on WPE: https://www.w3schools.com/jsref/tryit.asp?filename=tryjsref_onclick_html

[Carlos Alberto Lopez Perez]

The previous example is not working because of the frames. WPE is not passing the event correctly to the subframes.

Another example that is also not working: https://www.w3schools.com/jsref/tryit.asp?filename=tryjsref_onwheel_addeventlistener

[Carlos Alberto Lopez Perez]

Created attachment 318837 [details]
Patch

[Michael Catanzaro]

Comment on attachment 318837 [details]
Patch

Good work, but it seems now the files are identical except for EventHandler::shouldTurnVerticalTicksIntoHorizontal, right? Or nearly so? We should merge these files and use #if PLATFORM(GTK) where necessary to avoid duplicate code.

[Carlos Alberto Lopez Perez]

Created attachment 318844 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 318844 [details]
Patch

Clearing flags on attachment: 318844

Committed r221075: <http://trac.webkit.org/changeset/221075>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Garcia Campos]

Comment on attachment 318844 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318844&action=review

> Source/WebCore/PlatformGTK.cmake:149
> +    platform/glib/EventHandlerGlib.cpp

I think we agreed on using GLib instead of Glib, at least for new files. But anyway, I don't think GLib is a good name for this file, since it doesn't use glib at all, and it was not GTK because it used GTK+, but because it was the GTK port implementation of event handler.

[Carlos Alberto Lopez Perez]

(In reply to Carlos Garcia Campos from comment #7)
> Comment on attachment 318844 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318844&action=review
> 
> > Source/WebCore/PlatformGTK.cmake:149
> > +    platform/glib/EventHandlerGlib.cpp
> 
> I think we agreed on using GLib instead of Glib, at least for new files. But
> anyway, I don't think GLib is a good name for this file, since it doesn't
> use glib at all, and it was not GTK because it used GTK+, but because it was
> the GTK port implementation of event handler.

I agree that GLib is likely not the best name, but I don't have a better proposal.

What would be a good name in your opinion?

[Carlos Garcia Campos]

(In reply to Carlos Alberto Lopez Perez from comment #8)
> (In reply to Carlos Garcia Campos from comment #7)
> > Comment on attachment 318844 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=318844&action=review
> > 
> > > Source/WebCore/PlatformGTK.cmake:149
> > > +    platform/glib/EventHandlerGlib.cpp
> > 
> > I think we agreed on using GLib instead of Glib, at least for new files. But
> > anyway, I don't think GLib is a good name for this file, since it doesn't
> > use glib at all, and it was not GTK because it used GTK+, but because it was
> > the GTK port implementation of event handler.
> 
> I agree that GLib is likely not the best name, but I don't have a better
> proposal.
> 
> What would be a good name in your opinion?

Maybe Linux, but it could be that a windows implementation using GTK+ would be the same as the current GTK port implementation, so I'm not sure...

[Michael Catanzaro]

I think GLib is the best suffix to use for these cases. We all agree the file doesn't use GLib, but it seems like the best suffix to use for WPE + GTK.

[Carlos Alberto Lopez Perez]

(In reply to Carlos Garcia Campos from comment #7)
> > Source/WebCore/PlatformGTK.cmake:149
> > +    platform/glib/EventHandlerGlib.cpp
> 
> I think we agreed on using GLib instead of Glib, at least for new files.

I was not aware of this, I just followed (what it looked to me) the pattern the other files there followed.

I can upload a follow-up patch renaming this (or maybe upload a more general patch renaming everything to avoid future confusions like this one)

[Carlos Garcia Campos]

(In reply to Carlos Alberto Lopez Perez from comment #11)
> (In reply to Carlos Garcia Campos from comment #7)
> > > Source/WebCore/PlatformGTK.cmake:149
> > > +    platform/glib/EventHandlerGlib.cpp
> > 
> > I think we agreed on using GLib instead of Glib, at least for new files.
> 
> I was not aware of this, I just followed (what it looked to me) the pattern
> the other files there followed.
> 
> I can upload a follow-up patch renaming this (or maybe upload a more general
> patch renaming everything to avoid future confusions like this one)

I don't think it makes sense to rename all files, we should just make sure we follow the rule for new files. Since this one is still new, feel free to rename it in an unreviewed commit.

[Carlos Alberto Lopez Perez]

Committed r221260: <http://trac.webkit.org/changeset/221260>