GNOME Bugzilla – Bug 619421
Margin preview icon in Page setup dialog is not generated using the available space for it
Last modified: 2010-05-26 07:56:22 UTC
The size values that the margin preview icon is generated from is hard coded. As font size, and other factors change relating to the surrounding widget sizes, this icon will not scale appropriately.
Created attachment 161774 [details] [review] Proposed patch to fix this
Review of attachment 161774 [details] [review]: + guint widths[4]; + for (i = 0; i < table->ncols; i++){ + widths[i] = 0; + } While at this time table->ncols may be 4 or less this invites potential trouble later on. similarly or heights[9]. GValue is an opaque structure (and GTK has been known to change things that weren't even so marked.) So + GValue *top_att, *bottom_att, *left_att, *right_att; ... + if ((uint) left_att + 1 == (uint) right_att){ is completely unacceptable. You need to use g_value_get_uint (). Personally I would prefer margin_preview_page_available_size to return void and take an MarginPreviewPageAvailableSize* argument.
Created attachment 161817 [details] [review] Revised patch I have made the following changes to deal with the points above: 1) guint widths and heights arrays are now 20 items long. 2) No longer use GValue pointers to access container properties. 3) margin_preview_page_available_size now returns void and takes an MarginPreviewPageAvailableSize* argument.
Review of attachment 161817 [details] [review]: 1) guint widths and heights arrays are now 20 items long. the same argument applies. While it may be less likely to run out of space, you should really allocate exactly as much space as is really needed. (g_new) 2) No longer use GValue pointers to access container properties. It hadn't even occurred to me that you were effectively casting gint to GValue * and then back to guint... "top-attach" and friends properties are gint not guint, you should use the right type.
Created attachment 161856 [details] [review] Revised patch #2 1) Done. I use g_new0 so that I don't have to initialize the array to zeros, and I clean up with g_free. 2) According to: http://library.gnome.org/devel/gtk/2.20/GtkTable.html "top-attach" and friends properties are guint.
Created attachment 161857 [details] [review] Patch #2 Botched upload - trying again.
You are of course right that "top-attach" and friends properties are guint. When I looked them up I ended up at the child properties of gtk_menu that have the same name.
When I enlarge the page setup dialog the portrait/landscape/reverse landscape, etc items move apart. Since you made sure that the margin spin buttons did not move apart it would be nice if you could do the same for the orientation/center items.
Review of attachment 161857 [details] [review]: nice work (but there are still many other little things to improve with the page setup dialog)
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
... dialog-printer-setup.c:84: error: expected specifier-qualifier-list before 'uint' dialog-printer-setup.c: In function 'margin_preview_page_available_size': dialog-printer-setup.c:412: error: 'MarginPreviewPageAvailableSize' has no member named 'width' dialog-printer-setup.c:413: error: 'MarginPreviewPageAvailableSize' has no member named 'height' dialog-printer-setup.c:445: error: 'uint' undeclared (first use in this function) dialog-printer-setup.c:445: error: (Each undeclared identifier is reported only once ...
Hmm, I don't see that.
Nor do I. Morten, does your compiler support 'uint' at all? The only thing that I can see that may be a problem is that i used uint instead of guint. I'm busy putting together a patch that replaces uint with guint, and fixes the items from comment 8.
I have changed the uint to guint which might fix what you are seeing (and which it should have been in the first place). Please check again.
Sameer, Mingw32 does not support uint, so we should not be using it.
Created attachment 161987 [details] [review] Patch to glade file that deals with items moving apart + clipping bug 1) As per comment 8, this patch disables the portrait/landscape/reverse landscape,etc items from moving apart. 2) Work around a weird clipping bug with an embedded canvas widget in a table cell whose child's packing properties is not set to fill. This seems to be triggered when the margin preview widget draws the landscape page preview.
Review of attachment 161987 [details] [review]: committed as is