GNOME Bugzilla – Bug 710238
Fix the margin in RTL
Last modified: 2013-11-15 01:58:08 UTC
The behavior of the margin-left property and the margin-right property in RTL in wrong. The margin-right property in LTR adds margin on right side. In RTL I want to add this margin to the opposite direction - to left. The margin-left property in LTR adds margin on left side. In RTL I want to add this margin to the opposite direction - to right. This mean when the developer adds a margin with margin-right, he need to check the direction of the widget, and if it RTL - use margin-left instead margin-right. And vice versa. For examle: if (gtk_widget_get_direction (widget) == GTK_TEXT_DIR_RTL) gtk_widget_set_margin_left (widget, 5); else gtk_widget_set_margin_right (widget, 5); I think on two solutions: - To change the behavior of the margin-left property and the margin-right property, to check the direction, and to set the margin to the right direction. margin-left will add margin on left side in LTR and margin on right side in RTL. margin-right will add margin on right side in LTR and margin on left side in RTL. - To add margin-start and margin-left, to handle also in RTL. margin-start will add margin on left side in LTR and margin on right side in RTL, margin-left will add margin on right side in LTR and margin on left side in RTL. It requires to add also gtk_widget_{get,set}_{start,end} functions, and to recommend (in the document) to use margin-{start,end} instead of margin-{right,left}.
Created attachment 257444 [details] [review] GtkWidget: Add margin-start and margin-end properties
Review of attachment 257444 [details] [review]: ::: gtk/gtkwidget.c @@ +14147,3 @@ + else + return _gtk_widget_get_aux_info_or_defaults (widget)->margin.left; +} I would use an auxiliary for the value returned by _gtk_widget_get_aux_info_or_defaults here
Created attachment 257607 [details] [review] GtkWidget: Add margin-start and margin-end properties
I think if we're deprecating left/right and go with start/end, we should make those the defaults in the aux struct and do the if (direction == LEFT) thing in get/set_left/right_margin. Also, did you make sure to fix all GTK code to not use the deprecated functions (or properties in UI files)?
Review of attachment 257607 [details] [review]: ::: gtk/gtkwidget.c @@ +14185,3 @@ + *start = margin; + gtk_widget_queue_resize (widget); + g_object_notify (G_OBJECT (widget), "margin-start"); it would probably be technically more correct to also notify margin-left / -right, whichever it ends up to be. @@ +14245,3 @@ + *end = margin; + gtk_widget_queue_resize (widget); + g_object_notify (G_OBJECT (widget), "margin-end"); Same here
(In reply to comment #4) > I think if we're deprecating left/right and go with start/end, we should make > those the defaults in the aux struct and do the if (direction == LEFT) thing in > get/set_left/right_margin. Sure? It will broke some margin, for example in epiphany: https://git.gnome.org/browse/epiphany/tree/src/ephy-toolbar.c#n185 > Also, did you make sure to fix all GTK code to not use the deprecated functions > (or properties in UI files)? Of course.
(In reply to comment #6) > Sure? > It will broke some margin, for example in epiphany: > https://git.gnome.org/browse/epiphany/tree/src/ephy-toolbar.c#n185 > What I mean is that there should be a variable aux_info->margin.start instead of aux_info->margin.left
Created attachment 258343 [details] [review] GtkWidget: Add margin-start and margin-end properties
Attachment 258343 [details] pushed as 9921bec - GtkWidget: Add margin-start and margin-end properties
Created attachment 259843 [details] [review] Replace all margin-left and margin-right with margin-start and margin-end
Comment on attachment 259843 [details] [review] Replace all margin-left and margin-right with margin-start and margin-end Attachment 259843 [details] pushed as 719dd63 - Replace all margin-left and margin-right with margin-start and margin-end