Created attachment 48620 [details]
screenshot to show how the message headers should be aligned

adding i18n keyword

confirming the bug

how does the code check for the locale direction?

/* Reading directions for text */
typedef enum
{
  GTK_TEXT_DIR_NONE,
  GTK_TEXT_DIR_LTR,
  GTK_TEXT_DIR_RTL
} GtkTextDirection;

/* Functions for setting directionality for widgets*/
void             gtk_widget_set_direction         (GtkWidget        *widget,
                                                   GtkTextDirection  dir);
GtkTextDirection gtk_widget_get_direction         (GtkWidget        *widget);
void             gtk_widget_set_default_direction (GtkTextDirection  dir);
GtkTextDirection gtk_widget_get_default_direction (void);


Basically to fix the problem you simply need to not set the TextDirection to LTR
and let it be NONE, which would resolve to the locale direction.

that's no good, it isn't a gtk-widget, it is html.

we need to get the value to set however one gets that.

still in 2.7.4. :-/

The code in em-format-html.c is self contained at cannot tell whether the top level gtk_widget is RTL or LTR.

Some struct has to be changed to get this done, by including an GtkTextDirection or is_rtl variable. May be _EMFormatHTML in em-format-html.h, but I don't know where to inject code that checks for it.

This problem appears in many places in evolution (see bug #391408 for example). I think the correct solution is to make GtkHtml direction aware so things like gtk_widget_set_direction work, and so it can inherit the direction of containing widgets.

Any thoughts on the above suggestions?

Makes sense to me.

Created attachment 102090 [details] [review]
Fix message headers

This patch fixes the message headers as well as the signature message. 

I worked on gnome-2-20 and then moved my changes to trunk, but I have only tested on gnome-2-20.

Please test and let me know if OK. I will attach a screenshot of what it should look like.

Created attachment 102091 [details]
Screenshot of fixed headers.

Note the message headers.

Not the alignment of the signature message (broken) down the Screenshot.

Setting align="right" is not enough.  I'd also set style="dir: rtl".

Created attachment 102092 [details] [review]
Updated patch

Behdad, on what element? I set that attribute on the table containing the headers.

Updated patch also swaps "nowrap" attributes and aligns the header names to the left in order to be consistent. New screenshot will be attached.

Created attachment 102093 [details] [review]
Updated patch...

Sorry for the bug message spam.

Created attachment 102094 [details]
Updated screenshot of new view.

Humm, what if you just set table to 100% width and set dir=rtl on it?  In regular HTML that's all you need IIRC.  or maybe set align=right there too.  The rest should be simply inherited automatically.

Also, I prefer that the header values be right-aligned instead of left.  Think Persian/Arabic Subject lines and sender/recipients names.

dir=rtl does nothing.

Where did you get it anyway? I can't find any GtkHTML reference documentation or DTD :-(

For alignment, There are two fields: header titles and header values.

Optimal solution:

If I could get the table to go 100% then I believe the ideal solution it to have the two fields aligned to the far left and far right: that will make reading them easier, and it looks pleasing.

Currently the table is already set to 100% width, so I'm not sure what wrong. I'll fiddle more.

Other solution:

Header titles: my first screenshot had them aligned to the right, the second they are aligned to the left. I can't make up my mind on this, but if we go by optimal shown above, I think they better go to the far right.

Header values: Most of the time, the header values are latin, almost 100% of the time for Mailer, 
From (email address), and Date (set by headers...), they'd look a bit hard to read at first glance if their starting points are not aligned.

Humm, how about style="direction: rtl"?

Created attachment 102143 [details] [review]
Testing rtl align

I tried:

dir=rtl
direction=rtl
style="dir: rtl"
style="direction: rtl"

None work.  May be I am doing something wrong? my test is attached.

No, it just means that GtkHTML doesn't support it.

Created attachment 102174 [details] [review]
Updated patch.

I have tried various views and came down to this last solution.

This is pretty optimal IMO, and should go into trunk. I will attach a screenshot of the view.

Created attachment 102175 [details]
Screenshot of final view.

Yes, that's quite nice now.

Also you should be able to avoid code duplication by doing things like:

  camel_stream_printf ("blah blah %s blah blah", is_rtl ? "right" : "left");

There are lots of subtle changes you haven't noticed that'll make that rather verbose: note the th/td changes, the nowrap change, the order of the arguments...

+ I wanted to preserve the original formatting.

Djihen, 
a) You have there one non-call of gtk_widget_get_default_direction (you missed brackets there, around line 1850).
b) From my point of view, the TH is table header and TD is table "data", so placing table header under table data is very unusual. Are you sure with the validity of such construction?
c) Please, add ChangeLog entry too. Thanks in advance.

gtkhtml/htmlengine.c has code for parsing the "dir" attribute in various elements and setting some internal state indicating the text direction.  I'm not sure what it does with it beyond that, but maybe it could be made to work correctly without too much trouble.

Example from htmlengine.c:

static void
element_parse_html (HTMLEngine *e, HTMLObject *clue, const char *str)
{
    HTMLElement *element;
    char *value;

    element = html_element_new_parse (e, str);

    if (element) {
        if (e->parser_clue && html_element_get_attr (element, "dir", &value)) {
            if (!g_ascii_strcasecmp (value, "ltr"))
                HTML_CLUEV (e->parser_clue)->dir = HTML_DIRECTION_LTR;
            else if (!g_ascii_strcasecmp (value, "rtl"))
                HTML_CLUEV (e->parser_clue)->dir = HTML_DIRECTION_RTL;
        }

        html_element_free (element);
    }
}

Created attachment 102340 [details] [review]
Fixed patch

Milan: 

a) Thanks for the catch. Fixed.
b) The W3C recommends that but it doesn't specifically say it has to be that way.
http://www.w3.org/TR/html401/struct/tables.html#h-11.2.6
But I fixed that anyway.
c)Done.

to Matt, it changes the direction of text, but it doesn't change the order of columns, so it is somehow easier as Djihed did it.

to Djihed, I see there some minor coding style issues, like
d) we write "if (expr) {" (notice spaces around)
e) if you call function, there is a space between function name and open bracket too ("fun (params)")
f) do not add white spaces on the line, there was a cleanup of such things recently (empty line should be empty line, not line with white space characters)

ad b) ahh, I got the point why you did it in this way now. You've right, it should be <TD><TH> for rtl. I'm sorry I forced you to change it.

If you can fix these little things, then you can commit to trunk. (Do you have rights for it?)

Milan, thanks for the comments. I fixed those issues and committed under revision 34780.

*** Bug 391407 has been marked as a duplicate of this bug. ***