After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 394182 - Massive code cleanup
Massive code cleanup
Status: RESOLVED FIXED
Product: GtkHtml
Classification: Other
Component: Editing
3.13.x
Other Linux
: High normal
: ---
Assigned To: gtkhtml-maintainers
Evolution QA team
evolution[codecleanup]
Depends on:
Blocks:
 
 
Reported: 2007-01-08 10:14 UTC by Matthew Barnes
Modified: 2007-08-13 04:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (193.24 KB, patch)
2007-01-08 10:16 UTC, Matthew Barnes
none Details | Review
Updated patch for gtkhtml-3.13.5 (194.69 KB, patch)
2007-01-09 02:21 UTC, Matthew Barnes
committed Details | Review
proposed gtkhtml patch (2.07 KB, patch)
2007-08-08 12:53 UTC, Milan Crha
committed Details | Review

Description Matthew Barnes 2007-01-08 10:14:52 UTC
I took some time over the holidays to clean up GtkHtml's code a bit.  The patch turned out to be ~5000 lines, so my apologies in advance to whomever winds up reviewing this.

Here's what I changed [1]:


- Fix (almost) all compiler warnings.

    Obsoletes bug #363036.

    Excludes warnings caused by bug #363033.


- Stop using deprecated GTK+ widgets and symbols.

    NOTE: There's still a few spots where deprecated GTK+ symbols are
          being used, so we can't compile with -DGTK_DISABLE_DEPRECATED
          just yet.  In particular, construction of the editor's toolbar
          involves calls to things like gtk_toolbar_append_widget().
	  Also, htmlselect.c uses the deprecated GtkCombo widget, but I
          don't yet understand that code well enough mess with it.

    Use GtkComboBox instead of GtkOptionMenu [2].

    Use g_object_force_floating() instead of GTK_OBJECT_SET_FLAGS().

    Use g_object_is_floating() instead of GTK_OBJECT_FLOATING().

    Use g_object_ref_sink() instead of gtk_object_sink().

    Use g_idle_add() instead of gtk_idle_add().

    Use g_timeout_add() instead of gtk_timeout_add().

    Use g_source_remove() instead of gtk_idle_remove().

    Use g_source_remove() instead of gtk_timeout_remove().

    Use gtk_widget_set_size_request() instead of gtk_widget_set_usize().

    Use gtk_window_set_resizable() instead of gtk_window_set_policy().


- Stop using _some_ deprecated libgnomeui widgets and symbols.

    Use GtkColorButton instead of GnomeColorPicker.

    Use gtk_window_set_icon_name() instead of
        gnome_window_icon_set_from_file().


- Stop including <gnome.h>

    <gnome.h> includes <libgnomeui/libgnomeui.h> which in turn uses
    the deprecated GtkCombo widget.  In many cases, <gtk/gtk.h> is all
    that's really needed.  When libgnomeui symbols are needed, include
    only the necessary header files (e.g. <libgnomeui/gnome-app-helper.h>).


- Prefer stock or named icons.

    GtkHtml still distributes many icon files that are now provided by
    gnome-icon-theme.  The following is a list of GtkHtml icon files and
    their equivalent stock or named icons which are now used in the code.
    I removed these icon files from art/Makefile.am so they will no longer
    be distributed, and I propose removing them from Subversion as well.

      GtkHtml Icon File                 Replaced With
      ------------------------------    ------------------------------
      art/16_copy.png                   GTK_STOCK_COPY
      art/24_copy.png                   GTK_STOCK_COPY
      art/16_cut.png                    GTK_STOCK_CUT
      art/24_cut.png                    GTK_STOCK_CUT
      art/16_paste.png                  GTK_STOCK_PASTE
      art/24_paste.png                  GTK_STOCK_PASTE
      art/font-tt-16.png                "stock_text-monospaced"
      art/font-tt-24.png                "stock_text-monospaced"
      art/insert-image-16.png           "stock_insert_graphic"
      art/insert-image-24.png           "stock_insert_graphic"
      art/insert-link-16.png            "stock_insert-url"
      art/insert-link-24.png            "stock_insert-url"
      art/insert-object-16.png          "stock_insert_graphic"
      art/insert-object-24.png          "stock_insert_graphic"
      art/insert-rule-16.png            "stock_insert-rule"
      art/insert-rule-24.png            "stock_insert-rule"
      art/insert-table-16.png           "stock_insert-table"
      art/insert-table-24.png           "stock_insert-table"
      art/properties-16.png             GTK_STOCK_PROPERTIES
      art/redo-16.png                   GTK_STOCK_REDO
      art/redo-24.png                   GTK_STOCK_REDO
      art/search-16.png                 GTK_STOCK_FIND
      art/search-24.png                 GTK_STOCK_FIND
      art/search-and-replace-16.png     GTK_STOCK_FIND_AND_REPLACE
      art/search-and-replace-24.png     GTK_STOCK_FIND_AND_REPLACE
      art/smiley-1.png                  "stock_smiley-6"
      art/smiley-2.png                  "stock_smiley-5"
      art/smiley-3.png                  "stock_smiley-1"
      art/smiley-4.png                  "stock_smiley-3"
      art/smiley-5.png                  "stock_smiley-2"
      art/smiley-6.png                  "stock_smiley-4"
      art/smiley-8.png                  "stock_smiley-8"
      art/smiley-9.png                  "stock_smiley-9"
      art/smiley-10.png                 "stock_smiley-10"
      art/smiley-11.png                 "stock_smiley-11"
      art/smiley-12.png                 "stock_smiley-26"
      art/table-column-16.png           "stock_select-column"
      art/table-row-16.png              "stock_select-row"
      art/table-table-16.png            "stock_select-table"
      art/undo-16.png                   GTK_STOCK_UNDO
      art/undo-24.png                   GTK_STOCK_UNDO


[1] The capplet code appears to be dead.  It's not distributed in GtkHtml
    releases, and the latest capplet/ChangeLog entry is dated 2004-07-22.
    Therefore I didn't bother with it.  This bug only applies to the
    libgtkhtml and html-editor source code.

[2] The paragraph style combo box was particularly fun to migrate.
    Since some of its items are desensitized when in plaintext mode,
    a simple text combo box won't do.  Instead, we have to set up a
    two-column list store to populate the combo box widgets and manage
    the sensitivity of their items.  I wound up splitting the logic
    for this widget into new source files:

        components/html-editor/paragraph-style.[ch]
Comment 1 Matthew Barnes 2007-01-08 10:16:41 UTC
Created attachment 79730 [details] [review]
Proposed patch
Comment 2 Matthew Barnes 2007-01-09 02:21:56 UTC
Created attachment 79813 [details] [review]
Updated patch for gtkhtml-3.13.5
Comment 3 Srinivasa Ragavan 2007-01-10 05:08:59 UTC
I'll review this asap.
Comment 4 Kjartan Maraas 2007-01-25 15:33:46 UTC
I think we should take care of the patches that add features or fix bugs before applying a huge patch that potentially breaks all other patches...
Comment 5 Matthew Barnes 2007-01-25 15:48:24 UTC
I agree.  I'll try to keep the patch up-to-date until the time is right to consider applying it.  I'm already shipping it in Fedora "Rawhide" to give it some field testing.
Comment 6 Srinivasa Ragavan 2007-01-28 18:52:48 UTC
Matthew, It took me really long time to just go through it once. Sorry about that. Felt that quite a lot of code are pretty straight forward. I dont think I can be 100% sure of the patch. I think we should push this to HEAD to get it more tested. It is really difficult to review such a HUGE patch to 100% confidence :( Hope you can understand the point. Please push this to next HEAD build.

  
Comment 7 Matthew Barnes 2007-01-28 21:50:01 UTC
Sure, I understand completely.  FWIW, this has been in Fedora "Rawhide" for a few weeks now and I've not heard any complaints.  I'll try to partition future efforts like this into less overwhelming chunks.

One question before I commit this.  Should I also remove the old GtkHtml icon files listed in comment #0 from Subversion?  Those icons are now provided by gnome-icon-theme, and I don't see much point in keeping them around.
Comment 8 Matthew Barnes 2007-01-29 04:12:29 UTC
Committed to HEAD.

I removed the obsolete icon files.  If this is a problem I can add them back easy enough, but I see no need for them to remain under source control.
Comment 9 Matthew Barnes 2007-01-29 12:44:10 UTC
Kjartan Maraas noticed a small error in my patch:

Index: toolbar.c
===================================================================
--- toolbar.c   (revision 8364)
+++ toolbar.c   (working copy)
@@ -61,7 +61,7 @@ active_font_size_changed (GtkComboBox *c
 
 static void
 font_size_changed (GtkHTML *html,
-                   GtkHTMLParagraphStyle style,
+                   GtkHTMLFontStyle style,
                    GtkHTMLControlData *cd)
 {
        if (style == GTK_HTML_FONT_STYLE_DEFAULT)

I'm going to go ahead and commit the change as part of this bug.
Comment 10 Milan Crha 2007-08-08 12:51:13 UTC
During my investigations on bug #426496 I was pointed by 'rodo' to see at this bug too, because this patch causes that bug. And I think there are few places where the replace from sink to ref_sink isn't right, it could leak. See patch.
Comment 11 Milan Crha 2007-08-08 12:53:08 UTC
Created attachment 93271 [details] [review]
proposed gtkhtml patch

it's the addon for the previous patch, I hope I did it right.
Comment 12 Srinivasa Ragavan 2007-08-09 07:05:31 UTC
Matthew, can you review/approve this?
Comment 13 Matthew Barnes 2007-08-10 15:58:39 UTC
Looks okay to me.  Thanks for catching those errors, Milan.
Comment 14 Johnny Jacob 2007-08-13 04:48:50 UTC
Committed revision 8520.