Moving all the RTL flipping bugs to the 2.4.0 milestone; in some
cases the changes should be easy to do. (GtkPaned comes to mind)
In those cases, we can move the bugs back to 2.2.0 if patches occur.

Created attachment 11818 [details] [review]
first step - flippability for accel labels

Created attachment 11835 [details] [review]
remaining stuff: flippability for menuitem, checkmenuitem, radiomenuitem, imagemenuitem,  tearoffmenuitem

Created attachment 11838 [details] [review]
flip submenu cascading

The last patch does not only change the cascading of submenus, but
also makes first-level submenus right-aligned with the menubar-item
which popped them up in RTL mode. 
This exposes a bug, which can be seen in testgtk with this patch applied:

Enable RTL mode (in "flipping")
Start "Item Factory" example. Select "Preferences". Note that the
submenu doesn't come up properly right-aligned. It is too far to the
right, the difference being approx. the size of the submenu
indicators. This problem occurs only for the Preferences menu and only
on the first mapping. Inserting printfs in
gtk_menu_item_position_menu() confirms that
GTK_WIDGET(menu)->requisition.width is smaller on the first mapping
than on the second mapping of the menu. 

The bug is not specific to the RTL code, it can also be exposed in LTR
mode: Position the "Item Factory" example over the right edge of the
screen, so that only the submenu indicators of the "Preferences" menu
would be offscreen. Then pop the "Preferences" menu up for the first
time. It is partially offscreen, because the calculation to detect
offscreenness is done with the too small requisition.width. On the
second mapping, the menu is moved to the left to bring it completely
on screen, as intended.

the accel label patch looks good to me.

in gtkimagemenuitem, you have a left-over:

+      g_print ("toggle_size %d width %d\n", GTK_MENU_ITEM
(image_menu_item)->toggle_size, width);

As a general style thing, I might write:

+      if (gtk_widget_get_direction (widget) == GTK_TEXT_DIR_RTL) 
+	x = widget->allocation.width - GTK_CONTAINER
(image_menu_item)->border_width -
+	  widget->style->xthickness - 
+	  GTK_MENU_ITEM (image_menu_item)->toggle_size +
+	  (GTK_MENU_ITEM (image_menu_item)->toggle_size - width) / 2;
+	else
+	  x = GTK_CONTAINER (image_menu_item)->border_width +
+	       widget->style->xthickness +
         (GTK_MENU_ITEM (image_menu_item)->toggle_size - width) / 2;

As something like:

      border = (GTK_CONTAINER (image_menu_item)->border_width +
                widget->style->xthickness + 
                (GTK_MENU_ITEM (image_menu_item)->toggle_size - width)
/ 2;
      if (gtk_widget_get_direction (widget) == GTK_TEXT_DIR_LTR)
        x = border;
      else
        x = widget->allocation.width - border - GTK_MENU_ITEM
(image_menu_item)->toggle_size;

To make reduce the duplication of code between the two cases.
(I also slightly prefer having the LTR case first.) A slightly
different way of doing this to calculate 'x' for the right-to-left
case and flip it, using the general formula:

 item_x_flipped = overall_x + overall_width - (item_x - overall_x) -
item_width;

 So, you can write:

   x = .....;  /* Calculate LTR position leaving out overall_x */

   if (direction == GTK_TEXT_DIR_LTR)
     item_x = overall_x + x;
   else
     item_x = overall_x + overall_width - x - item_width;

The same as the above really, except s/border/x/, since 
overall_x == 0.

In gtkmenuitem.c, you have a misplaced { in:

+      if (menu_item->submenu && menu_item->show_submenu_indicator) {


The calculation:

+	  if (direction == GTK_TEXT_DIR_RTL) {
+	    arrow_x = x + 1;
+	    arrow_type = GTK_ARROW_LEFT;
+	  }
+	  else {
 	  arrow_x = x + width - 1 - arrow_size + (arrow_size - arrow_extent)
/ 2;
+	    arrow_type = GTK_ARROW_RIGHT;
+	  }

Looks suspicious to me. Shouldn't the centering factor 
(arrow_size - arrow_size_extent) / 2 be in the RTL case as well?
Again, I think it makes sense to treat it as "base calculation
+ flipping".

 arrow_offset = 1 + (arrow_size - arrow_extent) / 2;
 if (direction == GTK_TEXT_DIR_LTR)
   arrow_x = x + width - arrow_offset - arrow_size;
 else
   arrow_x = x + arrow_offset;


In gtktearoffmenuitem.c, the calculation:

+	      if (direction == GTK_TEXT_DIR_RTL) {
+		arrow_x = x + width - 2 * ARROW_SIZE + ARROW_SIZE / 2; 
+		arrow_type = GTK_ARROW_RIGHT;	    
+	      }
+	      else {
 	      arrow_x = ARROW_SIZE / 2;
+		arrow_type = GTK_ARROW_LEFT;
+	      }
 	      x += 2 * ARROW_SIZE;

The x += ARROW_SIZE doesn't look for me for the RTL case.

The RTL-cascade patch looks fine.

Is the bug you are reporting here related to bug 80764? I don't
think so offhand, but it's sort of similar.

Yes, looks like it is the same problem as in bug 80764. 
At least I see that the requisition width of the "Preferences" menu
grows between the first and the second mapping because the
accel_string_width of the contained accellabels goes from 0 to 18...

I will append further diagnosis to bug 80764. Thanks for the detailed
patch review, btw. Will provide corrected patches here as soon as the
accel label bug is fixed.

What the cascading bug doesn't get right is reacting to changes in the
text direction during the menubar lifetime, since the toplevel menu
items in the menubar set their submenu direction based on the text
direction at creation time. The submenu direction is then inherited
throughout the hierarchy. 

This may not be worth fixing, since text direction is normally constant, 
but a fix turns out to be relatively simple: just re-set the submenu
direction before popping up the submenu for toplevel menu items.

Created attachment 11861 [details] [review]
cascading again

I hesitate to bring it up, but for complete correctness the 
code probably also should reset the menu direction for children
of torn off menus. (if (GTK_IS_MENU (item->parent)) &&
menu->torn_off && !menu->active).

You could at least reproduce this bug in testgtk pretty 
trivially, though I have trouble imagining a real-life
case.

Created attachment 11881 [details] [review]
revised patch

We don't actually reset ->torn_off when popping up a menu
that is torn off, which is why you need the !menu->active
check as well.

I'd really like to see the GtkAccelLabel bug fixed in 
GtkAccelLabel rather than worked around here.

Hmm, testing it it looks to me like the arrows for RTL menu
items aren't quite positioned right ... the look too far to
the right.

I believe this is mostly fixed by the patch below. There is still a
small 1 pixel difference which I can't quite explain, but it looks ok
to me.

Created attachment 12075 [details] [review]
fix for drawing rtl submenu arrows

Created attachment 12076 [details]
ltr example

Created attachment 12077 [details]
rtl example

Committed to HEAD now.