901880 - Compile failure in nsColorPicker.cpp: error: ‘gtk_color_selection_dialog_get_color_selection’ was not declared in this scope

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Frank Wein [:mcsmurf]]

Since Bug 875753 was checked in, the SeaMonkey build fails on the official SeaMonkey Linux boxen with this error:
/usr/bin/ccache /tools/gcc-4.5/bin/g++ -o nsColorPicker.o -c -I../../dist/stl_wrappers -I../../dist/system_wrappers -include /builds/slave/c-cen-t-lnx-ntly/build/mozilla/config/gcc_hidden.h -DNATIVE_THEME_SUPPORT -DMOZ_GLUE_IN_PROGRAM -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL  -DSTATIC_EXPORTABLE_JS_API -DMOZ_SUITE=1 -DNO_NSPR_10_SUPPORT -DOS_POSIX=1 -DOS_LINUX=1  -DCAIRO_GFX -DMOZ_APP_NAME='"seamonkey"' -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/ipc/chromium/src -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/ipc/glue -I../../ipc/ipdl/_ipdlheaders  -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/widget/gtk2 -I. -I../../dist/include  -I/builds/slave/c-cen-t-lnx-ntly/build/objdir/mozilla/dist/include/nspr -I/builds/slave/c-cen-t-lnx-ntly/build/objdir/mozilla/dist/include/nss     -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/widget/gtk2/../xpwidgets -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/widget/gtk2/../shared -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/layout/generic -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/layout/xul/base/src -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/other-licenses/atk-1.0  -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/widget/gtk2/../shared/x11  -fPIC  -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -Wcast-align -gdwarf-2 -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -fno-tree-vrp -pthread -pipe  -DNDEBUG -DTRIMMED -gdwarf-2 -Os -freorder-blocks  -fomit-frame-pointer  -I/builds/slave/c-cen-t-lnx-ntly/build/objdir/mozilla/dist/include/cairo   -I/builds/slave/c-cen-t-lnx-ntly/build/mozilla/widget/gtk2/compat -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gtk-unix-print-2.0     -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MP -MF .deps/nsColorPicker.o.pp  /builds/slave/c-cen-t-lnx-ntly/build/mozilla/widget/gtk2/nsColorPicker.cpp
nsFilePicker.cpp
../../../../mozilla/widget/gtk2/nsColorPicker.cpp: In member function ‘virtual nsresult nsColorPicker::Open(nsIColorPickerShownCallback*)’:
../../../../mozilla/widget/gtk2/nsColorPicker.cpp:82:266: error: ‘gtk_color_selection_dialog_get_color_selection’ was not declared in this scope
../../../../mozilla/widget/gtk2/nsColorPicker.cpp: In member function ‘void nsColorPicker::ReadValueFromColorChooser(GtkWidget*)’:
../../../../mozilla/widget/gtk2/nsColorPicker.cpp:153:264: error: ‘gtk_color_selection_dialog_get_color_selection’ was not declared in this scope

From what I see a workaround has been implemented for old GTK versions like used on this Linux box (thanks for the workaround :). Somehow the definition from gtkcolorselectiondialog.h does not seem to get used though. I've noticed that the gtkcolorselectiondialog.h file does not get included anywhere. Should it get included? I'm not really sure as other compat files like for example gtkplug.h also do not get explicitly included.

Comment 1

[Arnaud Bienner]

I'm not sure either how that works exactly as, indeed, other compat files aren't included elsewhere.

Karl, do you have more information about how this works? And do you think I missed something in my patch?

Comment 2

[Karl Tomlinson (:karlt)]

The filename of the header needs to match that used by gtk.h.
In this case it needs to be gtkcolorseldialog.h not gtkcolorselectiondialog.h.

Comment 3

[Arnaud Bienner]

OK, thanks for the clarification.
I can take care of the fix if you want?

Comment 4

[Arnaud Bienner]

Attachment: Proposed patch

Does this look OK to you Karl?

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 786524 [details] [diff] [review]
Proposed patch

Yes.

Comment 6

[Frank Wein [:mcsmurf]]

Pushed to mozilla-central directly: https://hg.mozilla.org/mozilla-central/rev/2fcbc0a619f2
Patch is also needed on aurora (Bug 875753 landed before uplift).

Comment 7

[Carsten Book [:Tomcat]]

(In reply to Frank Wein [:mcsmurf] from comment #6)
> Pushed to mozilla-central directly:
> https://hg.mozilla.org/mozilla-central/rev/2fcbc0a619f2
> Patch is also needed on aurora (Bug 875753 landed before uplift).

Hey Frank, had to backout this change in https://hg.mozilla.org/mozilla-central/rev/47bd850cb89b since this caused on mozilla-central a bustage on the linux builders like https://tbpl.mozilla.org/php/getParsedLog.php?id=26251851&tree=Mozilla-Central

Comment 8

[Frank Wein [:mcsmurf]]

Attachment: Better patch

I think this one should fix the problem, wrong include was used. Currently I'm testing it on try server.

Comment 9

[Frank Wein [:mcsmurf]]

Attachment: Better patch

Comment 10

[Frank Wein [:mcsmurf]]

Pushed: http://hg.mozilla.org/integration/mozilla-inbound/rev/67adb1fa9936

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/67adb1fa9936

Comment 12

[Frank Wein [:mcsmurf]]

Comment on attachment 786802 [details] [diff] [review]
Better patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 875753
User impact if declined: Users with older GTK versions cannot compile Gecko anymore
Testing completed (on m-c, etc.): Works fine on mozilla-central
Risk to taking this patch (and alternatives if risky): almost zero risk
String or IDL/UUID changes made by this patch: -

Comment 13

[Frank Wein [:mcsmurf]]

Pushed: https://hg.mozilla.org/releases/mozilla-aurora/rev/a4722ef7ae61

Comment 14

[u279076]

Assuming no QA needed here. Please remove [qa-] from the whiteboard and add the verifyme keyword if this needs QA.