Bug 25807 – #ifdef properly the WebCore/platform/gtk/KeyboardCodes.h

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Fridrich Strba]

Some of the keyboard codes are already part of win32 api, so the WebCore/platform/gtk/KeyboardCodes.h does not compile since we are redefining already defined codes. Following patch ifdefs correctly the different defines for building well on Windows. We do exactly what keyboard codes for other platforms do.

[Fridrich Strba]

Created attachment 30357 [details]
patch that ifdefs properly the codes for windows platforms

[Holger Freyther]

Comment on attachment 30357 [details]
patch that ifdefs properly the codes for windows platforms

No, that is way too ugly this way. Please let us go back to the problem and then talk about solutions.

So your problem is:
   - Some VK_ are already there

My questions:
   - Why do you have a define for WIN_CE? There is no Gtk+ port to Windows CE?!

Proposed change:
   - Make the change with one #if !PLATFORM(GTK) block instead of squattering that ugly thing all around. Alternatively create a cleaned KeyboardCodesGtkWin32.h and include that from KeyEventGtk.cpp

[Fridrich Strba]

What I did here is exactly what is done in the qt platform KeyboardCodes.h

[Fridrich Strba]

Sorry, to answer your questions:
1) some of the VK_ are there in win32 api in winuser.h
2) the WIN_CE is there only to be compliant with what the other platforms that are used on Windows do.
3) I can make the proposed change, no problem, I can remove the WIN_CE define too. I was just wondering whether doing consistently the same thing in all places was not a superior solution.

[Holger Freyther]

(In reply to comment #3)
> What I did here is exactly what is done in the qt platform KeyboardCodes.h

Okay, could you create a patch that is moving this header file to WebCore/platform, change the Gtk+ buildsystem and remove the original Gtk+ header?

[Fridrich Strba]

Created attachment 30739 [details]
sharing the Qt KeyboardCodes.h and removing the Gtk+ KeyboardCodes.h

[Fridrich Strba]

OK, it seems like the svn diff does not handle nice svn mv. If landing, svn mv WebCore/platform/qt/KeyboardCodes.h WebCore/platform/KeyboardCodes.h; svn rm WebCore/platform/gtk/KeyboardCodes.h and then patch the WebCore/GNUmakefile.am and WebCore/ChangeLog

[Holger Freyther]

(In reply to comment #7)
> OK, it seems like the svn diff does not handle nice svn mv. If landing, svn mv
> WebCore/platform/qt/KeyboardCodes.h WebCore/platform/KeyboardCodes.h; svn rm
> WebCore/platform/gtk/KeyboardCodes.h and then patch the WebCore/GNUmakefile.am
> and WebCore/ChangeLog

Right, please use svn-create-patch. :)

[Fridrich Strba]

Created attachment 30766 [details]
using svn-create-patch

[Holger Freyther]

Comment on attachment 30766 [details]
using svn-create-patch


> +        The two KeyboardCodes.h files are basically identical and the
> +        qt one is properly #ifdef-ed for different win32 systems. Share
> +		them between Qt and Gtk implementations

tabs... the pre-commit hook will curse me :)

[Brent Fulgham]

Assign to me for landing.

[Brent Fulgham]

Landed in @r44291.