408823 - Killing GTK1, deprecated glib uses

Status: RESOLVED FIXED

Description

[Oleg Romashin (:romaxa)]

I guess would be better to remove all GTK1(MOZ_WIDGET_GTK) code,
MOZ_WIDGET_GTK2 defines, and deprecated gtk/glib functionality

Comment 1

[Oleg Romashin (:romaxa)]

Created attachment 293673 [details] [diff] [review]
MOZ_WIDGET_GTK, MOZ_WIDGET_GTK2 defines code

https://garage.maemo.org/svn/browser/mozilla/branches/MOZ_BUG408238_BRANCH/libgtkmozembed/debian/patches/Bug408823.killing.gtk1.diff

Comment 2

[Oleg Romashin (:romaxa)]

Created attachment 293674 [details] [diff] [review]
Deprecated glib functions

For the reference:

Need to be applied after "MOZ_WIDGET_GTK, MOZ_WIDGET_GTK2 defines code" patch

https://garage.maemo.org/svn/browser/mozilla/branches/MOZ_BUG408238_BRANCH/libgtkmozembed/debian/patches/Bug408823.OldGlib.diff

Comment 3

[Oleg Romashin (:romaxa)]

Comment on attachment 293673 [details] [diff] [review]
MOZ_WIDGET_GTK, MOZ_WIDGET_GTK2 defines code

Obsolete after relanding bug 326152

Comment 4

[Oleg Romashin (:romaxa)]

Christian, if you have any additions to the second patch put your updates..

Comment 5

[Marco Pesenti Gritti]

The patch came out a bit messy because of the mix of tab/spaces (inherited from the current code). This had been fixed on the microb branches but was lost when we reverted.

I suggest that we fix this gradually, while changing the code for other reasons. Can you please rework the patch to get rid of tabs in the code blocks you touch?

Comment 6

[Oleg Romashin (:romaxa)]

Created attachment 296143 [details] [diff] [review]
Fixed indentation

I was using 
:set ts=2 sw=2 sts=2 tw=80 et cindent
for indentation.

Comment 7

[Marco Pesenti Gritti]

Comment on attachment 296143 [details] [diff] [review]
Fixed indentation

r+ with the following nitpick. Please use "func_name()" consistently instead of "func_name ()".

Comment 8

[Marco Pesenti Gritti]

Comment on attachment 296143 [details] [diff] [review]
Fixed indentation

Looks like we don't need sr  here, so the patch can go in.

Comment 9

[Reed Loden [:reed] (use needinfo?)]

mpgritti,

I'm sorry, but I don't know who you are, nor are you listed as a peer for gtkmozembed at http://www.mozilla.org/owners.html#gtk-embedding-widget, so unless timeless or bsmedberg say it's ok, I'd like to have review from one of them.

Comment 10

[Doug Turner (:dougt)]

reed, thanks for your comment and concern.

marco is the owner of the gtkmozembed code.  brendan is going to update the right stuff when he gets to it.  there is no sr required on GTK specific stuff in gtkmozembed.  If you want, you can land this patch, or I can.

Comment 11

[Reed Loden [:reed] (use needinfo?)]

(In reply to comment #10)
> marco is the owner of the gtkmozembed code.  brendan is going to update the
> right stuff when he gets to it.

Ah, ok. Thanks for the update.

> there is no sr required on GTK specific stuff
> in gtkmozembed.  If you want, you can land this patch, or I can.

As this code is part of the build of Firefox (my Linux build definitely builds gtkmozembed), it will need approval first before it can be landed, but I'll be glad to land it once it has approval.

Comment 12

[Doug Turner (:dougt)]

This code was backed out by me a few weeks ago.  I thought that there was a blanket a=, but lets ask anyway.

Comment 13

[Mike Schroepfer]

Comment on attachment 296143 [details] [diff] [review]
Fixed indentation

yep a+ let's make the paperwork match the reality.

Comment 14

[Reed Loden [:reed] (use needinfo?)]

Created attachment 296596 [details] [diff] [review]
unbitrotten patch with nit addressed

Comment 15

[Reed Loden [:reed] (use needinfo?)]

Checking in embedding/browser/gtk/src/EmbedContentListener.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedContentListener.cpp,v  <--  EmbedContentListener.cpp
new revision: 1.27; previous revision: 1.26
done
Checking in embedding/browser/gtk/src/EmbedEventListener.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedEventListener.cpp,v  <--  EmbedEventListener.cpp
new revision: 1.18; previous revision: 1.17
done
Checking in embedding/browser/gtk/src/EmbedProgress.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedProgress.cpp,v  <--  EmbedProgress.cpp
new revision: 1.26; previous revision: 1.25
done
Checking in embedding/browser/gtk/src/EmbedWindow.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedWindow.cpp,v  <--  EmbedWindow.cpp
new revision: 1.43; previous revision: 1.42
done
Checking in embedding/browser/gtk/src/EmbedWindowCreator.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedWindowCreator.cpp,v  <--  EmbedWindowCreator.cpp
new revision: 1.14; previous revision: 1.13
done
Checking in embedding/browser/gtk/src/gtkmozembed.h;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembed.h,v  <--  gtkmozembed.h
new revision: 1.47; previous revision: 1.46
done
Checking in embedding/browser/gtk/src/gtkmozembed2.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembed2.cpp,v  <--  gtkmozembed2.cpp
new revision: 1.64; previous revision: 1.63
done