User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.8b2) Gecko/20050302 Firefox/1.0+ Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.8b2) Gecko/20050302 Firefox/1.0+ currently Firefox can detect if the locale is RTL, and if such, change the theme for RTL compatibility. this is not yet applied in Thunderbird. Reproducible: Always the link for the bug in Firefox: http://bugzilla.mozilla.org/show_bug.cgi?id=221824

Note that as tb doesn't use winstripe, there is nothing we can take from bug 221824...

Reuven, can you please identify the Thunderbird icons that need RTL versions?

i'm attaching a list of files that can be modified for RTL interface.

this list includes the main files in use that were added in winstripe as well. can you please add them?

this patch mainly deals with changing the settings padding(margin)-right(left) to -moz-margin(padding)-end(start)to be compatible with rtl interface.

Reuven, are you working on it these days? we're going to have a real problem in thunderbird 1.1 (with the new update system)

(In reply to comment #6) > Reuven, are you working on it these days? we're going to have a real problem in > thunderbird 1.1 (with the new update system) the patch above resolves a big part of the theme. there are still some issues that need to get fixed, but these will be easier to identify once this patch is commited. also i need kevin to add the files listed in comment 4.

these files are needed for the RTL UI.

this patch fixes the margins and paddings to be compatible with RTL UI.

this patch fixes the account central icons to be RTL compatible. this patch requires patch 198606, in order to take effect.

Comment on attachment 198606 [details] [diff] [review] fixing margins and padding This was boring ;). r=mano, need moa from mscott and some trunk baking.

Comment on attachment 198610 [details] [diff] [review] fixing the account central direction > <page > xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" > xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" >- onload="OnInit();"> >+ onload="OnInit();" >+ style="direction: &locale.dir;;"> leave that part for the other bug. r=mano.

same as above, removing the page direction style.

Comment on attachment 199023 [details] [diff] [review] fix account central direction, with page direction removed r=mano

Reuven, can you give me a single cvs diff patch with all of mano's review comments for the CSS changes + your zip file with the new rtl files to add? That way I can check this into the trunk after I've finished reviewing it. Thanks.

this is a complete patch, including the changes in the patches above, and addressing mano's comments. i added in this patch a fix for the wizard-header in the import wizard(the initial wizard when a new profile is created). the RTL files are included in attachment 198147 [details] (fourth attachment).

seems like i removed icon64.png from jar.mn in the previous patch...

Too late for major theme changes. The time for something like this was in the alpha cycles, or possibly an early beta, but not right before RC1.

We'll figure something out here. No one panic. First step is for me to get this on the trunk. Another question, what is the impact to pinstripe here? is there any?

Well, at some point, i plan to work on rtl pinstripe, not a priority though (there's no native RTL UI in OS X).

Comment on attachment 198147 [details] [fixed on trunk]rtl files to add I'm about to land this on the trunk.

Comment on attachment 199190 [details] [diff] [review] [fixed on trunk]complete patch for rtl theme, fixed jar.mn I'm about to land this on the trunk. Will incorporate any additional feedback from Asaf. My concern about pinstripe was that some of these non theme specific CSS changes (like migration.css) would cause things to break in pinstripe because we don't have the right artwork.

Scott, note this doesn't seem to include attachment 198948 [details] [diff] [review].

(In reply to comment #24) > Scott, note this doesn't seem to include attachment 198948 [details] [diff] [review] [edit]. That attachment isn't even part of this bug :) I'll take a look.

You know, I'm really not happy to see the "localized" artworks. This is Scott's and MoFo/MoCo's decision though. At the very list, you've to fix the credits background too (and probably icon64.png, not an issue though since it's only used in Pinstripe).

(In reply to comment #26) > You know, I'm really not happy to see the "localized" artworks. This is Scott's > and MoFo/MoCo's decision though. > Which images are you worried about here Asaf. I had assumed the menu icons in the zip file were from the firefox RTL winstripe theme. The official branding artwork changes looked like our official art but flipping the orientation which looked ok to me. Can you point me to some images your not happy with?

Reuven, how is this looking on the trunk now that it's all checked in? We don't have the ability to verify these changes ourselves, we need your help.

(In reply to comment #27) > Which images are you worried about here Asaf. I had assumed the menu icons in > the zip file were from the firefox RTL winstripe theme. The official branding > artwork changes looked like our official art but flipping the orientation which > looked ok to me. Can you point me to some images your not happy with? I referred to the flipped branding artworks. Esp. the difference this creates between the about/credits dialog to the other "branded ui" bits (and the official website).

seems like i didn't add the fix for jar.mn inside toolkit/themes/qute/global. I'm also adding the fix for msgAccountCentral.xul from the previous patch. i tested the theme and it looks O.K from the RTL side as well, once the arrows are added and the msgAccountCentral.xul is fixed.

Comment on attachment 199190 [details] [diff] [review] [fixed on trunk]complete patch for rtl theme, fixed jar.mn retroactive r=mano pending a resolution on the artworks thingy.

(In reply to comment #29) > (In reply to comment #27) > > > I referred to the flipped branding artworks. Esp. the difference this creates > between the about/credits dialog to the other "branded ui" bits (and the > official website). Are you worried about the images being flipped or you don't think they look as good as the non RTL branded artwork?

The Former.

Reuven, can you attach a screen shot of the about dialog with the RTL theme so I can see what Asaf is worried about. Thanks!

Scott, the about dialog is the only place where the image _isn't_ flipped with this patc.

the about page looks quiet well in the hebrew version, and is also a problem to flip since it contains "Mozilla Thunderbird" in it. i will also attach the other windows with the images *not* fliped, to see if we prefer to leave the branding the images untouched.

this is the generic about dialog with the original image. seems to fit well with the hebrew text since it's on top of the text.

I'm ok with leaving the about dialog image alone. I think it looks fine in Reuven's about dialog screen shot. So the question becomes, do we leave the icon alone for the start page and the import dialog too? Or do we use the new rtl versions that went into the trunk. I'd like to land this on the branch today if we can figure out what the right thing to do here is.

Reuven, what did you do in the customized 1.0 theme?

there is no reason to flip logos for rtl builds. the images that are most important to be flipped are those that will change their meaning when they are flipped, which are mostly those with arrows on them (reply, forward etc.). lightning may also be a reason for flipping, although my position is that the light source should be from the top-left in rtl builds as well. the logo should be kept as in ltr builds.

the import dialog image looks pretty good in both directions. the start page more concers me, since the image goes above the text in some cases with the original image, like when the window is not maximized, or the resolution is 800X600. i'm attaching a screenshot of the start page with the image in both directions. with 800X600 resolution the image goes also above the h1, and h2 title backgrounds. also the start page is used quiet a lot (actually, every time the client is opened) so it's more important to have it look good. regarding the import image, if we keep the flipped image in the start page, i think we can leave the import rtl-flipped-image as well without it being an exception. we also need attachment 199680 [details] [diff] [review] checked in, in order for the theme to be RTL compatible.

I checked in the msgAccountCentral change on the trunk. That was just a missed checkin from earlier. We need to wrap this up today so I can get this onto the branch. How can we resolve a decision on the branding art? I don't have an opinion one way or the other really. Although life would be easier if we didn't have to have rtl specific versions of any of the branded art. We could leave the official artwork alone everywhere but the start page (i.e. make that the only rtl image we use). Or we could leave that alone too and let some of the text overlap like the screen shot shows?

My position/opinion is that the branding art should not be altered for RTL.

reuven, what's the connection between flipping the image and having it overlap the text? its just a bitmap, and a pretty symetric one too (circular). if it's not overlapping the text in the "rtl" (top) form in your lat attachment, there is no reason for it to overlap in it's original form.

This patch removes the official branded rtl artwork, making the rtl theme use our regular official branded artwork. As benjamin and Tsahi pointed out, I think this is the direction we should go in.

(In reply to comment #47) > Created an attachment (id=200141) [edit] > back out the official branding RTL artwork from the trunk > > This patch removes the official branded rtl artwork, making the rtl theme use > our regular official branded artwork. in migration.css we still need to align the image to the left in the RTL theme like this: .wizard-header { background: url("chrome://branding/content/thunderbird-wizard-badge.png") no-repeat right center Window; } .wizard-header[chromedir="rtl"] { background: url("chrome://branding/content/thunderbird-wizard-badge.png") no-repeat left center Window; }

(In reply to comment #47) > Created an attachment (id=200141) [edit] > back out the official branding RTL artwork from the trunk > > This patch removes the official branded rtl artwork, making the rtl theme use > our regular official branded artwork. this patch removes the startpage-back-rtl.png from start.xhtml instead of the thunderbird-watermark-rtl.png image. I've added attachment 200193 [details] [diff] [review] in Bug 311241 that removes the branding reference and also uses the -moz styles instead of margin-right-left styles.

Ok, this patch should address your comments. I took your patch from the other bug. Let's keep all the patches here in this bug so I can preserve my sanity. I'm already having a hard time keeping track of all the different RTL patches across the bugs :)

Ok, I landed everything on the branch for thunderbird 1.5. Please help test this for me on an hourly windows build when it comes out or with tomorrow's nightly build. With all these patches, I'm sure I forgot something somewhere and we'll need to react quickly if I did. :)

How's this looking on today's branch build? Reuven, are you happy with what made it in? Did we miss anything?

(In reply to comment #52) > How's this looking on today's branch build? Reuven, are you happy with what made > it in? Did we miss anything? tested on trunk - version 1.6a1 (20051022) tested on branch - version 1.5 (20051021) and looks o.k in both directions (RTL and LTR) it seems like actually everything is there :-)

> tested on trunk - version 1.6a1 (20051022) > tested on branch - version 1.5 (20051021) > > and looks o.k in both directions (RTL and LTR) this also includes the start page :-)

this patch fixes the back and next arrows that are on the primary toolbar.

(In reply to comment #55) > Created an attachment (id=200501) [edit] > fix for previous and next arrows > this patch fixes the back and next arrows that are on the primary toolbar. any chance to get this on the branch?

sorry man this patch came in after the deadline, it's too late for 1.5.

Comment on attachment 200501 [details] [diff] [review] fix for previous and next arrows r=mano if this is still relevant.

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).