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 92212 - When running in RTL mode the borders are not reversed
When running in RTL mode the borders are not reversed
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
2.10.x
Other Linux
: Normal normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks: 387893 Hebrew
 
 
Reported: 2002-08-31 23:14 UTC by Kenneth Rohde Christiansen
Modified: 2007-07-26 01:37 UTC
See Also:
GNOME target: ---
GNOME version: 2.9/2.10


Attachments
RTL border layout. (19.33 KB, image/png)
2007-03-08 19:46 UTC, Djihed Afifi
  Details
RTL png image... (19.33 KB, image/png)
2007-03-08 19:49 UTC, Djihed Afifi
  Details
automatically reverse window frame's buttons order (4.92 KB, patch)
2007-04-18 17:04 UTC, Yair Hershkovitz
rejected Details | Review
automatically reverse window frame's buttons order (5.57 KB, patch)
2007-06-11 08:33 UTC, Yair Hershkovitz
committed Details | Review

Description Kenneth Rohde Christiansen 2002-08-31 23:14:37 UTC
When running in RTL mode like when you run arabic, the border is not reversed.
Comment 1 Heath Harrelson 2002-10-30 15:45:27 UTC
Batch adding GNOME2 keyword to Metacity bugs.  Sorry for the spam.
Comment 2 Heath Harrelson 2002-10-30 20:00:14 UTC
Kenneth -- what exactly do you mean by the borders not being reversed?
 Could you supply a screenshot if that would be easier to understand?
Comment 3 Kjartan Maraas 2005-01-19 19:30:39 UTC
I guess he meant that the window buttons (close, maximize, minimize etc) should
be reversed.
Comment 4 Josh Lee 2005-03-11 21:08:05 UTC
Just make it so that when you parse /apps/metacity/general/button_layout, the
computed value is the reverse of the inputted value. Though I'm not sure if it's
such a good idea...
Comment 5 Thomas Thurman 2006-08-27 21:44:52 UTC
Can we find whether we have consensus on whether reversing button order is a good idea for RTL languages before we implement anything?
Comment 6 Behdad Esfahbod 2007-03-08 18:41:39 UTC
Not only the buttons.  The entire window header (or whatever it's called).  App icon should be on the right, and buttons on the left...  Just, you know, invert it.
Comment 7 Djihed Afifi 2007-03-08 19:45:46 UTC
Pinged quite a few maintainers/translators from various teams and it seems they are all for reversing it. A gimped sample of what it should look like reversed will be attached.
Comment 8 Djihed Afifi 2007-03-08 19:46:24 UTC
Created attachment 84264 [details]
RTL border layout.
Comment 9 Djihed Afifi 2007-03-08 19:49:22 UTC
Created attachment 84267 [details]
RTL png image...

Ah really sorry, that is an image, not a patch. You can ignore me for the rest of week now.
Comment 10 Yair Hershkovitz 2007-03-11 14:03:55 UTC
While doing the reverse border work - 
There is a need to add a generic text direction support: e.g
metacity_get_direction().

This kind of API can be used by other RTL bugs in metacity, for example:
http://bugzilla.gnome.org/show_bug.cgi?id=387893
Comment 11 Yair Hershkovitz 2007-04-15 15:38:34 UTC
I believe the fix should be to move the default button layout to be translated and translate to "close,maximize,minimize:menu" in RTL .po files.

This is a really small fix in the schema files + translators work.

However, this will only work for new users. We also need a solution to change the gconf button_layout value for existing users.
Comment 12 Behdad Esfahbod 2007-04-15 23:21:16 UTC
NO.  No reason for .po hacks.
Comment 13 Yair Hershkovitz 2007-04-18 17:04:43 UTC
Created attachment 86580 [details] [review]
automatically reverse window frame's buttons order

This patch fix both gconf and no-gconf modes.

Also fix the way the theme engine reads the button's functions. In its previous implementation the no-gconf mode wasn't working correctly.

Instead of going over all the position in the buttons array - stop when the LAST_FUNCTION value is reached (that's why it was called LAST_FUNCTION).
This also eliminates the hack to fill the buttons array with LAST_FUNCTION values.
Comment 14 Yair Hershkovitz 2007-04-18 17:07:04 UTC
And to think that it only took 5 years to create a patch :)

Regarding the schemas thought, it's not good because then the inversion won't be automatic.
Comment 15 Yair Hershkovitz 2007-04-18 19:14:40 UTC
Almost forgot. This patch requires infrastructure work from:
http://bugzilla.gnome.org/attachment.cgi?id=86429&action=diff
Comment 16 Yair Hershkovitz 2007-04-30 07:29:52 UTC
Ping
Comment 17 Thomas Thurman 2007-05-22 21:32:00 UTC
It looks generally good; I was worried that left_buttons and right_buttons would be partially uninitialised, but you caught that in theme.c. I need to review your patch to bug 387893 before I can properly test it, of course, so I'll go do that now.
Comment 18 Thomas Thurman 2007-05-23 14:35:47 UTC
So, the things I'm testing for:
  * In LTR, left goes to the left workspace and right to the right; in RTL vice versa
  * In LTR, popping up the window menu with the keyboard puts it on the LHS of the window; in RTL the RHS
  * In LTR, the window menu is flush with the screen to the left; in RHS to the right
  * Tooltips are right-justified in RTL and left-justified in LTR. 
Comment 19 Thomas Thurman 2007-05-23 15:01:21 UTC
Oops, wrong bug. My apologies.
Comment 20 Thomas Thurman 2007-05-27 05:42:07 UTC
I've had the chance to look over the patch now. These are the problems I've found:

1) (trivial) It doesn't, in fact, handle the case when GConf is in use at all. init_button_layout(); needs to be called when HAVE_GCONF is defined as well as at present when it isn't, or (better) it needs to be called outside the #ifdef entirely. This is trivially fixed.

2) (more serious) You determine whether we're in LTR or RTL mode using meta_ui_get_direction(), a function added in the fix to bug 387893, which uses  gtk_widget_get_default_direction() to determine its result.  gtk_widget_get_default_direction() gets its data from an internal variable, which defaults to LTR when a program first runs. In RTL locales, it is set by a call to gtk_widget_set_default_direction(), which is called from gtk_get_option_group(), which is called from gtk_parse_args(), which is called from gtk_init_check(), which is called from meta_ui_init(). So any attempt to check the RTL/LTR status before meta_ui_init() is called is hopeless. But meanwhile, you're attempting to make use of it in init_button_layout(), which you call from meta_prefs_init(). But meta_prefs_init() gets called six lines *before* meta_ui_init() in main(). So I'm not sure how this patch could ever have worked!

(Obviously we could try switching round meta_ui_init() and meta_prefs_init() in main(), though that might have other subtle side-effects.)
Comment 21 Yair Hershkovitz 2007-05-27 09:41:02 UTC
I will attach a fixed patch asap. hopefully not more than a week.
Comment 22 Behdad Esfahbod 2007-05-28 06:22:12 UTC
Isn't gtk_widget_get_default_direction() initialized from the locale (using translations).
Comment 23 Yair Hershkovitz 2007-05-28 17:04:07 UTC
Behdad: yes, but's as thomas mentioned - that's triggered in meta_ui_init()

thomas: 1) you are incorrect. when gconf is enabled the button layout is taken from the conf entry. an initial value is meaningless, it is ignored.

2) i guess that it is not possible to call meta_ui_init() before meta_prefs_init(). However, i don't see any problem taking out gtk_init() from meta_ui_init() and putting it before meta_prefs_init().

Another possibility (which requires more work) is to make no changes when loading the buttons layout. instead do the reverting upon drawing the buttons.

It's your call, i can handle both ways.

As for how it worked, maybe there update conf calls that made miss this problem.
Or maybe my test environment was evil :)
Comment 24 Thomas Thurman 2007-05-28 17:46:05 UTC
1) Okay, but then how does this handle the problem raised in comment 10 about users who are using an RTL locale, who upgrade from a previous version of Metacity and who have the buttons in the "wrong" order?

2) I think moving the gtk_init() to main() before meta_prefs_init() sounds a good plan. Anyone else want to comment on the pros and cons of this?
Comment 25 Yair Hershkovitz 2007-06-11 08:33:00 UTC
Created attachment 89730 [details] [review]
automatically reverse window frame's buttons order

Now meta_ui_init() and meta_errors_init() are called before meta_prefs_init()
Comment 26 Yair Hershkovitz 2007-06-11 08:36:13 UTC
Fixed patch is available.

1) I don't see any problem raised in comment 10 . If the buttons are read from 
gconf, then it will be automatically reversed. If metacity was compiled with no gconf (have this ever happend?) then the rtl defaults will be used.

2) I've tool a deeper look and there was no problem with moving the whole meta_ui_init() befire neta_prefs_init().

This time i tested with no environment hacks.
Comment 27 Thomas Thurman 2007-06-11 11:12:01 UTC
Thanks for your patch!

1) I apologise: I meant to say comment 11. I think you do in fact handle this case (from looking at the patch; I haven't tested it yet because I'm away from my development environment at the moment).

In fact metacity is designed to work without gconf because of use in embedded systems. I don't have any information to hand as to how many people actually use it this way.

2) Thanks for looking into this.

When I get my hands on my development environment again (probably after work tonight) I'll test it and let you know whether it's okay to commit.
Comment 28 Yair Hershkovitz 2007-07-01 14:14:14 UTC
Any progress with the testing?
Comment 29 Thomas Thurman 2007-07-01 16:04:10 UTC
Totally fell off the radar (new job, etc); thanks for the nudge. I will try to fix this tonight.
Comment 30 Yair Hershkovitz 2007-07-22 06:44:34 UTC
Commited.

2007-07-22  Yair Hershkovitz  <yairhr@gmail.com>

        Reverse window buttons and align them to the left for RTL locales.
        Fixed #92212.

        * src/prefs.c (button_layout, init_button_layout, update_button_layout):
        Support reversing and left-aligning of buttons for both Gconf and
        NO-Gconf modes. 
        * src/main.c (main): Call meta_ui_init() before meta_prefs_init().
        meta_prefs_init() check for RTL locales which is initialized in
        meta_ui_init().
        * src/theme.c (meta_frame_layout_calc_geometry): Fixed access to
        button_layout to stop iterating when getting to a
        META_BUTTON_FUNCTION_LAST value.
Comment 31 Thomas Thurman 2007-07-23 01:13:57 UTC
Thanks.
Comment 32 Yair Hershkovitz 2007-07-24 12:39:44 UTC
Thomas, please close this bug. I don't have sufficient permissions to do it myself.
Comment 33 Thomas Thurman 2007-07-24 13:13:58 UTC
Sure, sorry. Done.
Comment 34 Elijah Newren 2007-07-26 01:37:07 UTC
[Excersizing role as bugsquadder...] Yair: You can edit bugs now.  :-)