Bug 84929

Created attachment 138935 [details] Patch

The gamepad module uses "float[]" syntax in its Gamepad.idl. The JSC binding generator doesn't compile "float[]" well now, the issue is being discussed in the bugs 80696 84540. So this bug depends on the bugs 80696 84540.

Comment on attachment 138935 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138935&action=review > Source/WebCore/CMakeLists.txt:2376 > +IF (ENABLE_GAMEPAD) You also need to add an entry to cmakeconfig.h.cmake. > Source/WebCore/CMakeLists.txt:2388 > + Modules/gamepad/NavigatorGamepad.cpp > + Modules/gamepad/Gamepad.cpp > + Modules/gamepad/GamepadList.cpp Please keep this list sorted.

Created attachment 139122 [details] Patch

Comment on attachment 139122 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139122&action=review Just a missing change, looks good otherwise. > Source/WebCore/GNUmakefile.list.am:1042 > IDL_PATH should also be updated, it's in GNUmakefile.am.

CCing Zan, who should be interested for the GTK port side of this :)

Created attachment 143725 [details] Patch:add changing Source/WebCore/GNUmakefile.am Adding gamepad to IDL_PATH

Comment on attachment 143725 [details] Patch:add changing Source/WebCore/GNUmakefile.am View in context: https://bugs.webkit.org/attachment.cgi?id=143725&action=review Other than the following comments this looks OK for the GNUmakefiles. > Source/WebCore/GNUmakefile.am:807 > + $(WebCore)/Modules/gamepad \ A few more changes are required in GNUmakefile.am: - '-I$(srcdir)/Source/WebCore/Modules/gamepad \' include path should be added to webcore_cppflags - '$(shell ls $(srcdir)/Source/WebCore/Modules/gamepad/*.idl) \' file listing should be added to EXTRA_DIST > Source/WebCore/GNUmakefile.list.am:1057 > + Source/WebCore/Modules/gamepad/Gamepad.cpp \ > + Source/WebCore/Modules/gamepad/Gamepad.h\ > + Source/WebCore/Modules/gamepad/GamepadList.cpp \ > + Source/WebCore/Modules/gamepad/GamepadList.h\ > + Source/WebCore/Modules/gamepad/NavigatorGamepad.cpp \ > + Source/WebCore/Modules/gamepad/NavigatorGamepad.h A few more changes are required in GNUmakefile.list.am: - 'DerivedSources/WebCore/JSGamepad.(cpp|h)' and 'DerivedSources/WebCore/JSGamepadList.(cpp|h)' should be added to webcore_built_sources - '$(WebCore)/Modules/gamepad/(Gamepad|GamepadList|NavigatorGamepad).idl' entries should be added to dom_binding_idls

Created attachment 143750 [details] Patch

(In reply to comment #9) > Created an attachment (id=143750) [details] > Patch What's the difference with previous patch? Zan's review doesn't seem to be adressed in this new patch.

Created attachment 144009 [details] Patch Sorry, I uploaded the wrong patch, and didn't check after uploading.

Comment on attachment 144009 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144009&action=review > Source/WebCore/GNUmakefile.list.am:1064 > + Source/WebCore/Modules/gamepad/NavigatorGamepad.h Missing backslash here

Since the work has somewhat ceased in here I went on and added necessary files and directory inclusions to the patch that's up for review in bug #87503. Because of that I'm also removing the block flag for that bug. Other than that, the latest patch should just remove any modifications to GNUmakefiles and it'd probably be ready to land.

Comment on attachment 144009 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144009&action=review > Source/WebCore/CMakeLists.txt:2402 > +IF (ENABLE_GAMEPAD) I don't like to use macro both CMake and files. Files related to Gamepad are already guarded by #if ENABLE(GAMEPAD). Isn't this duplicating macro usage ?

Looks like this has been implemented through various other bugs.