I haven't tracked it down yet, but it appears that the problem is not directly related to rtl locales - the positional button functions (left_right_background, right_left_background) appear to be broken for any button order not being minimize,maximize,close - e.g. changing the layout to :close,minimize,maximize messes up the prelight/pressed states as well.

Created attachment 175686 [details] [review]
buttons: Fix background functions for non-default layouts

While the configured layout is taken into account for positioning
the buttons, the mapping from button function states to button
position states just assumed the default button layout in LTR
locales.
Do a proper mapping depending on the actual layout instead.

I'm not entirely happy with the patch, but putting up for comments anyway:
 1. It feels like duplicating code (even though strictly speaking
    it doesn't)
 2. The handling of the (LEFT|RIGHT)_MIDDLE_BACKGROUND functions is not
    correct when used more than once - the problem is that this case is
    handled for positioning, but not for states.
    (But then, it's not worse than before ...)

Created attachment 175690 [details] [review]
buttons: Fix background functions for non-default layouts

Actually got it wrong and had to swap two constants ...

Created attachment 175747 [details] [review]
buttons: Fix background functions for non-default layouts

Moved the mapping from button function states to button position states entirely into ui/theme.c - looks a bit cleaner to me.

Review of attachment 175747 [details] [review]:

::: src/ui/frames.c
@@ -2402,3 @@
-  /* Map button function states to button position states */
-  button_states[META_BUTTON_TYPE_LEFT_LEFT_BACKGROUND] =
-    button_states[META_BUTTON_TYPE_MENU];

Somehow the code here is nonsense and nobody has noticed for a long time. There can be more than one button of type left_middle_background/right_middle_background so keeping track of button state by the "button position" button types doesn't make sense.

In my opinion, meta_frame_layout_calc_geometry() needs to save the MetaButtonLayout that is passed in the MetaFrameLayout() and then the button states should be passed in to 
meta_frame_style_draw_with_style() only for the real button type, and not the background. When drawing a real button, we can then use the saved MetaButtonLayout to map.

Created attachment 176024 [details] [review]
buttons: Fix background functions for non-default layouts

Updated to save the layout and do the mapping in meta_frame_style_draw_with_style().

Review of attachment 176024 [details] [review]:

In frames.c

  MetaButtonLayout button_layout;
  
[...]

  meta_prefs_get_button_layout (&button_layout);
  
  meta_theme_calc_geometry (meta_theme_get_current (),
                            type,
                            frame->text_height,
                            flags,
                            width, height,
                            &button_layout,
                            fgeom);

So,

fgeom->button_layout = (MetaButtonLayout *)button_layout;

Looks really suspicious to me, like we're saving a pointer to an automatic variable. It would seem more reasonable to me to have fgeom have a by-value MetaButtonLayout and do:

 fgeom->button_layout = *button_layout

Otherwise looks fine for me except for a small comment

::: src/ui/theme.c
@@ +4261,3 @@
+    /* Map position buttons to the corresponding function */
+    case META_BUTTON_TYPE_RIGHT_LEFT_BACKGROUND:
+      function = fgeom->button_layout->right_buttons[0];

Shouldn't you check 

 if (fgeom->n_right_buttons > 0)

as you do elsewhere? Seems to be as necessary here.

@@ +4272,3 @@
+      break;
+    case META_BUTTON_TYPE_LEFT_LEFT_BACKGROUND:
+      function = fgeom->button_layout->left_buttons[0];

As above

Created attachment 177517 [details] [review]
In reply to comment #7)

> Looks really suspicious to me, like we're saving a pointer to an automatic
> variable. It would seem more reasonable to me to have fgeom have a by-value
> MetaButtonLayout and do:
> 
>  fgeom->button_layout = *button_layout

OK.


> Shouldn't you check 
> 
>  if (fgeom->n_right_buttons > 0)
> 
> as you do elsewhere? Seems to be as necessary here.

Yeah.

Review of attachment 177517 [details] [review]:

Looks good