Closed Bug 476423 Opened 14 years ago Closed 12 years ago

[RTL] Fennec must support rtl

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
major

Tracking

(fennec2.0b2+)

VERIFIED FIXED
Future
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: bugzilla, Assigned: jap)

References

Details

(Keywords: intl, rtl, Whiteboard: [fennec-checkin-posta1])

Attachments

(6 files, 12 obsolete files)

132.79 KB, image/png
Details
147.33 KB, image/png
Details
50.74 KB, image/png
Details
53.63 KB, image/png
Details
41.69 KB, image/png
Details
75.06 KB, patch
mbrubeck
: review+
Details | Diff | Splinter Review
I made Arabic language pack for Fennec:
http://people.mozilla.com/~dynamis/fennec/arlangpacktest/fennec-1.0a2.ar.langpack-0.0.2009020217.xpi
Install and chang "general.useragent.locale" to "ar" and run fennec.
Then you'll see broken fennec UI:
http://people.mozilla.com/~dynamis/fennec/arlangpacktest/screenshot.png

As you see, smartbar's items are ordered wrongly.
This is because current Fennec UI don't support ltr language properly I believe.

note: I also made Italian langpack in the same way and it works file:
http://people.mozilla.com/~dynamis/fennec/itlangpacktest/fennec-1.0a2.it.langpack-0.0.2009020218.xpi
http://people.mozilla.com/~dynamis/fennec/itlangpacktest/screenshot-1.png
http://people.mozilla.com/~dynamis/fennec/itlangpacktest/screenshot-2.png
http://people.mozilla.com/~dynamis/fennec/itlangpacktest/screenshot-3.png

and Japanese Langpack (original) works of course file:
http://ftp-developer.mozilla-japan.org/pub/mozilla-japan/fennec/nightly/langpack/

So, I believe broken UI of Arabic Fennec is caused by Fennec's bug not supporting ltr (not bug of my script to make langpack).
Flags: blocking-fennec1.0?
Summary: Fennec must support ltr → Fennec must support rtl
I meant rtl, not ltr. sorry.
Keywords: intl
I reproduced on Windows Vista (Vista itself is Japanese) too:
http://people.mozilla.com/~dynamis/fennec/arlangpacktest/screenshot.vista.png

on the other hand linostart say:
there is still no RTL support in the UI, but it's not broken as in your snapshot
http://www.khaledhosny.org/files/tmp/fennec1.png
http://www.khaledhosny.org/files/tmp/fennec2.png

So, this may be one of
 * bug only for win/mac
 * bug relating OS language
   # win/mac screenshot by me is taken on Japanese OS
   # linux screenshotmy linostart is taken on Arabic OS
Most probably it's a theme-related. In dynamis' shot there is an RTL support but the UI is broken. But in my shot, there is no RTL support because I've manually installed the langpack; when using the addon manager to install it, I got the same results as dynamis.

I believe now that some .css files content need to be modified.
You might be interested in the following article, which document some of the process used for the desktop applications. Fennec is based on the same platform, so it will be also useful for it.

https://developer.mozilla.org/En/Making_Sure_Your_Theme_Works_with_RTL_Locales


(I'm adding 'rtl' keyword to the bug)
Keywords: rtl
tracking-fennec: --- → 1.0+
Attached patch Patch v0.1 (obsolete) — Splinter Review
Add support for RTL for:
 * Toolbar
 * Sidebars
 * Right Panels
 * Checkbox

It's a first draft patch.
Attached patch Patch v0.2 (obsolete) — Splinter Review
Code clean up and CSS tweaks for checkbox
Attachment #384251 - Attachment is obsolete: true
Attached patch Patch v0.3 (obsolete) — Splinter Review
Small changes
Attachment #384268 - Attachment is obsolete: true
Attached patch Patch v0.3 (obsolete) — Splinter Review
Sorry for the spam, I've forgotten the BookmarkList...
Attachment #384311 - Attachment is obsolete: true
I think we could update the patch based on bug 478416. However, I'm not sure when that patch will land (and where)
Flags: blocking-fennec1.0?
I'll provide an updated patch :
 * based on 478416
 * Without altering the sidebars position
  Madhava, do we really need to switch their position depending on the locale direction?
Assignee: nobody → 21
Depends on: 478416
-ing this for 1.0
tracking-fennec: 1.0+ → 1.0-
(In reply to comment #16)
> -ing this for 1.0

Please note that unless Fennec will have some basic RTL support, it might be too broken in RTL language packs, and this will block our l10n efforts.
Anas, Tomer, could we get an update on how Fennec is doing with respect to RTL right now?

The dependent bugs are fixed, and this one is the only open bug with the rtl keyword set. Hard to triage right now, we do have builds for ar and he, but they're missing a good deal of strings right now, too.
Attached patch Patch v0.4 (obsolete) — Splinter Review
moz-locale-dir is awesome!

This patch add a basic RTL support for Fennec, i supposed some images are bad (go button for example) but this can be adressed as followup.
Attachment #384312 - Attachment is obsolete: true
Attachment #384315 - Attachment is obsolete: true
Attached patch Patch v0.5 (obsolete) — Splinter Review
some minimal changes
Attachment #399730 - Attachment is obsolete: true
(In reply to comment #20)
> Created an attachment (id=399732) [details]
> Patch v0.5
> 
> some minimal changes

Is there some test build available for testing? I hope this won't miss the 1.0 release.

Do you mind if I'll set it as 'wanted-fennec1.0?'?
I am using Mozilla/5.0 (X11; U; Linux i686;he; rv:1.9.2b1pre) Gecko/20091009 Fennec/1.0b4 and found that chrome://global/locale/global.dtd is hardcoded to 'ltr' because it retrive its data from /fennec/toolkit/chrome/en-US.jar instead of /fennec/chrome/he.jar. Currently this blocking me from fixing bug 522982, and might cause some troubles also for this patch.  

I see it is used by patch v0.5 here - https://bugzilla.mozilla.org/attachment.cgi?id=399732&action=diff#a/chrome/content/browser.xul_sec1
Comment on attachment 399732 [details] [diff] [review]
Patch v0.5

This patch is probably bitrotted now.

I need to check with Mark if the approach was ok, and in this case I'll provide an updated patch as soon as possible.
Attachment #399732 - Flags: review?(mark.finkle)
Tested rc1 - still having some RTL theme issues.
Tested with Arabic on Gecko/20100407 Namaroka/3.7a4pre Fennec/1.1a2pre on Mac OS X and the sidestrip for tabs can fill full screen. Couple of screenshots in http://www.flickr.com/photos/33996899@N00/sets/72157623794755302/
tracking-fennec: 1.0- → ?
Target Milestone: --- → Future
Blocks: 526477
This patch is an updated version of the previous one. It fixes some further style issues but doesn't fix the problem of "the sidestrip for tabs can fill full screen" (hint - this _might_ be due to the add-tab icon sticking to the far right). Further the settings-open-rtl-64.png does differ from the one in ltr (at that position) but was provided before - so I used it (just to mention it).
Blocks: 584225
Attachment #447479 - Attachment is obsolete: true
Attachment #462597 - Flags: review?(mark.finkle)
Comment on attachment 462597 [details] [diff] [review]
Update patch (and also add more support for rtl tabs)

># User Axel Linder <axel.linder.vc@googlemail.com>
>Bug 476423 - Fennec must support rtl
>diff -r d122fcec536b chrome/content/browser.js
>--- a/chrome/content/browser.js	Tue Aug 03 13:28:30 2010 -0700
>+++ b/chrome/content/browser.js	Tue Aug 03 16:44:07 2010 -0700
>@@ -979,8 +979,20 @@
>+    let leftbarCBR;
>+    let ritebarCBR;
>+
>+    // Workaround...
>+    // in case of rtl the containers are swapped...so we swap them here, too!
>+    if( (document.getElementById('tabs-container').getBoundingClientRect().left) > (document.getElementById('browser-controls').getBoundingClientRect().left) ) {
>+      // RTL
>+      ritebarCBR  = document.getElementById('tabs-container').getBoundingClientRect();
>+      leftbarCBR = document.getElementById('browser-controls').getBoundingClientRect();
>+    } else {
>+      // LTR
>+      leftbarCBR = document.getElementById('tabs-container').getBoundingClientRect();
>+      ritebarCBR = document.getElementById('browser-controls').getBoundingClientRect();
>+    }

Let's simplify that to something like this:

> let leftbarCBR = document.getElementById('tabs-container').getBoundingClientRect();
> let ritebarCBR = document.getElementById('browser-controls').getBoundingClientRect();
> if( (leftbarCBR..left > ritebarCBR.left) )
>   [ritebarCBR, leftbarCBR] = [leftbarCBR, ritebarCBR]; // RTL



>+          if ((document.getElementById('tabs-container').getBoundingClientRect().left) > (document.getElementById('browser-controls').getBoundingClientRect().left)) {
>+              let controlsWidth = document.getElementById('browser-controls').getBoundingClientRect().width;
>+              let completeWidth = controlsWidth + this.lastChild.boxObject.x + 8;
>+
>+              let boundingBox = this.firstChild.getBoundingClientRect();
>+              let columnsCount = Math.ceil(this.childNodes.length / Math.floor(this.style.height / boundingBox.height));
>+              if (this._columnsCount != columnsCount) {
>+                this.style.width = ( (this.firstChild.boxObject.x - completeWidth) + (2 * boundingBox.width) ) + "px";
>+                this._columnsCount = columnsCount;
>+              }
>+            } else {
>+              let boundingBox = this.firstChild.getBoundingClientRect();
>+              let columnsCount = Math.ceil(this.childNodes.length / Math.floor(this.style.height / boundingBox.height));
>+              if (this._columnsCount != columnsCount) {
>+                this.style.width = (this.lastChild.boxObject.x + boundingBox.width) + "px";
>+                this._columnsCount = columnsCount;
>+              }

Let's get rid of the duplicate code.  Something like this (untested):

>           let tabRect = document.getElementById('tabs-container').getBoundingClientRect();
>           let controlsRect = document.getElementById('browser-controls').getBoundingClientRect();
>           let rtl = (tabRect.left > controlsRect.left);
> 
>           let boundingBox = this.firstChild.getBoundingClientRect();
>           let columnsCount = Math.ceil(this.childNodes.length / Math.floor(this.style.height / boundingBox.height));
>           if (this._columnsCount != columnsCount) {
>             if (rtl) {
>               let completeWidth = controlsRect.width + this.lastChild.boxObject.x + 8;
>               this.style.width = ( (this.firstChild.boxObject.x - completeWidth) + (2 * boundingBox.width) ) + "px"; // rtl
>             } else {
>               this.style.width = (this.lastChild.boxObject.x + boundingBox.width) + "px";
>             }
>             this._columnsCount = columnsCount;
>           }

I'm worried about the magic "8" in there - is that for padding/margins?  Can we read that from the appropriate style instead?  W



>+#identity-box:-moz-locale-dir(rtl) {
>+  min-height: 64px;
>+  min-width: 64px;
>+  background: url("images/rightcap-default-64.png") top left no-repeat;
>+  -moz-box-align: center;
>+  -moz-box-pack: center;
>+}

Only override the "background" property, please - don't duplicate properties that you aren't changing.

> /* make sure this endcap matches the other endcap */
>-#urlbar-icons {
>+#urlbar-icons:-moz-locale-dir(ltr) {
>   min-height: 64px;
>   min-width: 64px;
>   background: url("images/rightcap-default-64.png") top right no-repeat;
>   -moz-box-pack: center;
> }
> 
>+#urlbar-icons:-moz-locale-dir(rtl) {
>+  min-height: 64px;
>+  min-width: 64px;
>+  margin-left: +8px;
>+  background: url("images/leftcap-default-64.png") top right no-repeat;
>+  -moz-box-pack: center;
>+}

Same here - leave the original ruleset alone, and just add a new one overriding the "background" property.

If you can, please use "-moz-margin-end" in a shared rule set instead of margin-left in a rtl rule set.

Remove the "+" from "+8px".

>-#tool-panel-open {
>+#tool-panel-open:-moz-locale-dir(ltr) {
>   list-style-image: url("chrome://browser/skin/images/settings-default-64.png");
>   margin-right: -40px; /* big number just to make sure the image overflows the edge */
> }
> 
>-#tool-panel-open:not([disabled="true"]):hover:active {
>+#tool-panel-open:-moz-locale-dir(rtl) {
>+  list-style-image: url("chrome://browser/skin/images/settings-default-64.png");
>+  margin-left: 0px; /* big number just to make sure the image overflows the edge */
>+}

The comment is no longer accurate.

Can you replace these with a single rule set using -moz-margin-end?

>+#tool-panel-open:not([disabled="true"]):hover:active:-moz-locale-dir(ltr) {
>+  list-style-image: url("chrome://browser/skin/images/settings-active-64.png");
>+}
>+
>+#tool-panel-open:not([disabled="true"]):hover:active:-moz-locale-dir(rtl) {
>   list-style-image: url("chrome://browser/skin/images/settings-active-64.png");
> }

Aren't these the same?

> /* button overflows off the left edge */
>-#tool-panel-close {
>+#tool-panel-close:-moz-locale-dir(ltr) {
>   list-style-image: url("chrome://browser/skin/images/settings-open-64.png");
>   margin-left: -40px; /* big number just to make sure the image overflows the edge */
> }
> 
>+#tool-panel-close:-moz-locale-dir(rtl) {
>+  list-style-image: url("chrome://browser/skin/images/settings-open-64.png");
>+  margin-right: 0px; /* big number just to make sure the image overflows the edge */

Same as above.

>-#tabs-container {
>+#tabs-container:-moz-locale-dir(ltr) {
>   -moz-padding-start: 4px; /* allow the thumbnails to get close to the edge */
>   -moz-padding-end: 8px; /* core spacing */
>   padding-bottom: 8px; /* core spacing */
>   border-right: 3px solid #262629;
> }
> 
>-#tabs {
>+#tabs-container:-moz-locale-dir(rtl) {
>+  display: moz-box;
>+  -moz-column-width: 128px;
>+  -moz-box-align: end;
>+  -moz-box-pack: end;
>+  -moz-padding-start: 4px; /* allow the thumbnails to get close to the edge */
>+  -moz-padding-end: 8px; /* core spacing */
>+  max-width: -moz-max-content;
>+  padding-bottom: 8px; /* core spacing */
>+  border-right: 3px solid #262629;
>+}

Keep the shared values in one ruleset.  You probably want -moz-border-end instead of border-right.

I don't understand why moz-box, moz-column-width, and max-width are needed in rtl only; can you explain?

>-#tabs {
>+#tabs:-moz-locale-dir(ltr) {
>   display: block;
>   -moz-column-width: 128px;
>   -moz-column-gap: 0;
>@@ -740,9 +813,20 @@
>   background-color: transparent;
> }
> 
>+#tabs:-moz-locale-dir(rtl) {
>+  display: block;
>+  -moz-column-width: 128px;
>+  -moz-column-gap: 0;
>+  -moz-user-focus: ignore;
>+  max-width: -moz-max-content;
>+  margin: 0;
>+  padding: 0;
>+  background-color: transparent;
>+}

Same as above - override only the properties that differ.  Why are padding/parming needed?

>-#tabs-controls {
>+#tabs-controls:{

This change is a syntax error.

>   margin-top: 8px; /* core spacing */
>-  -moz-box-pack: start;
>+  -moz-box-pack: end;
> }

I think -moz-back-pack should still be "start".  (You can see this once the selector is fixed, above.)

>diff -r d122fcec536b themes/core/jar.mn

The new image files are not included in this patch.  I assume they are just mirror images of the ltr versions?

> radio:not([disabled=true]):last-child:active:hover,
> radio:last-child[selected] {
>   -moz-border-image: url("chrome://browser/skin/images/toggleright-active-64.png") 8 repeat repeat;
> }
> 
>+radio:last-child[selected]:-moz-locale-dir(rtl) {
>+  -moz-border-image: url("chrome://browser/skin/images/toggleleft-active-64.png") 8 repeat repeat;
>+}

Looks like all of these radio styles fail to override the :active:hover styles.
Attachment #462597 - Flags: review?(mark.finkle) → review-
I agree with all of Matt's suggestions
Attached patch Updated patch. (obsolete) — Splinter Review
Fixed issues:

* Simplified Javascript
* Consistently use original ruleset + ruleset for rtl with only modified values
* Use -moz-margin-start/-moz-margin-end where possible instead of a rtl ruleset
* Remove not required #tabs-container, #tabs rules
* Fix syntax error and wrong rule in #tabs-controls
* Fix radio :active:hover styles
* Attached rtl icons to the patch (created with ImageMagick: convert -flop icon.png icon-rtl.png)
Attachment #462597 - Attachment is obsolete: true
Attachment #464973 - Flags: review?(mbrubeck)
tracking-fennec: ? → 2.0+
tracking-fennec: 2.0+ → 2.0b2+
Comment on attachment 464973 [details] [diff] [review]
Updated patch.

>+++ b/chrome/content/browser.js
>+    if (leftbarCBR.left > ritebarCBR.left) {
>+      // RTL
>+      [ritebarCBR, leftbarCBR] = [leftbarCBR, ritebarCBR];
>+    }

Please put the comment on the same line as the body, and remove curly braces.

>+++ b/chrome/content/browser.xul
>+              if (rtl) {
>+                width = firstBox.right - lastBox.left;
>+              }

Remove curly braces.

>+++ b/themes/core/browser.css
>+#urlbar-icons:-moz-locale-dir(rtl) {
>+  margin-left: 8px;
>+  background: url("images/leftcap-default-64.png") top right no-repeat;
>+}

Why is margin-left needed here?  It adds spacing that is not there in LTR.

>+++ b/themes/core/platform.css
> radio.checkbox-radio-on[selected] {
>   -moz-border-image: url("chrome://browser/skin/images/toggleon-active-64.png") 8 repeat repeat;
> }
> 
>+radio.checkbox-radio-on[selected]:-moz-locale-dir(rtl) {
>+  -moz-border-image: url("chrome://browser/skin/images/toggleon-active-rtl-64.png") 8 repeat repeat;
>+}
>+
> radio.checkbox-radio-off {
>   -moz-border-image: url("chrome://browser/skin/images/toggleoff-inactive-64.png") 8 repeat repeat;
> }
> 
> radio.checkbox-radio-off[selected] {
>   -moz-border-image: url("chrome://browser/skin/images/toggleoff-active-64.png") 8 repeat repeat;
> }

RTL rules are missing for .checkbox-radio-off and possibly some other radio styles.  (See screenshot.)

We need an RTL version of the dropmarker-* images for the menulist. (See screenshot.)

r=mbrubeck with those minor adjustments.  Looks great!
Attachment #464973 - Flags: review?(mbrubeck) → review+
A few more nits:

- margin-left and margin-right for #content-navigator .next-button and .previous-button should be changed to -moz-margin-start and -moz-margin-end.  (See screenshot.)

Need to swap the "arrowright" and "arrowleft" images in #form-helper-autofill for RTL, and change padding-left/border-left/border-right to start/end.

Probably need an RTL version of task-back-40.png, and the #tool-back and #tool-forward button icons.  (I *really* wish we could reverse images in CSS...)
Yes, I already started yesterday to fix also the smaller issues in #content-navigator and .autocomplete (there icons are shown on the wrong side in rtl).

I will update the patch later today or tomorrow to fix the minor problems from review and add more rtl fixes, which are either mentioned in comment 34 or which I found yesterday.
Attached patch Updated patch v2 (obsolete) — Splinter Review
Addressed all mentioned issues.

I also looked for some more rtl-issues in the css files and fixed them.
Attachment #464973 - Attachment is obsolete: true
Attachment #465474 - Flags: review?(mbrubeck)
Comment on attachment 465474 [details] [diff] [review]
Updated patch v2

This looks great, thanks!  (And now my brain hurts after testing Fennec in RTL mode.)
Attachment #465474 - Flags: review?(mbrubeck) → review+
Great! Let's hold off from checking in until alpha1 wraps up.
Whiteboard: [fennec-checkin-posta1]
Comment on attachment 465474 [details] [diff] [review]
Updated patch v2

This patch switch the right (tabs) and left (controls) sidebars of Fennec. I would like to heard about RTL's people if we want to switch the sidebars or if it is better to keep them as if?

>diff --git a/chrome/content/browser.js b/chrome/content/browser.js
>--- a/chrome/content/browser.js
>+++ b/chrome/content/browser.js
>@@ -1008,16 +1008,19 @@ var Browser = {
>     }
> 
>     if (!dx) dx = 0;
>     if (!dy) dy = 0;
> 
>     let leftbarCBR = document.getElementById('tabs-container').getBoundingClientRect();
>     let ritebarCBR = document.getElementById('browser-controls').getBoundingClientRect();
> 
>+    if (leftbarCBR.left > ritebarCBR.left)
>+      [ritebarCBR, leftbarCBR] = [leftbarCBR, ritebarCBR]; // switch in RTL case
>+

You could probably add a helper function into BrowserUI to know if the UI is RTL or not (see my patch version 3 or 4 I think)

>diff --git a/chrome/content/tabs.xml b/chrome/content/tabs.xml
>--- a/chrome/content/tabs.xml
>+++ b/chrome/content/tabs.xml
>@@ -121,20 +121,28 @@
>           ]]>
>         </body>
>       </method>
> 
>       <field name="_columnsCount">1</field>
>       <method name="_updateWidth">
>         <body>
>           <![CDATA[
>-            let boundingBox = this.firstChild.getBoundingClientRect();
>-            let columnsCount = Math.ceil(this.childNodes.length / Math.floor(this.style.height / boundingBox.height));
>+            let tabRect = document.getElementById('tabs-container').getBoundingClientRect();
>+            let controlsRect = document.getElementById('browser-controls').getBoundingClientRect();
>+            let rtl = (tabRect.left > controlsRect.left);
>+

Please use the helper to know if we are RTL or not and use this.parentNode if possible to avoid using document.getElementById("tabs-container");
Also please use " instead of ' :)

Thanks a lot for moving forward the patch and doing this work. It was kind of out of my sigh for a while!
(In reply to comment #39)
> This patch switch the right (tabs) and left (controls) sidebars of Fennec. I
> would like to heard about RTL's people if we want to switch the sidebars or if
> it is better to keep them as if?

Generally, it is preferred to swap all UI elements. However, I don't see much difference in either position here (since neither denote a start nor an end), but I'd prefer switching the sidebars to be on the safe side.
(In reply to comment #39)
> Comment on attachment 465474 [details] [diff] [review]
> Updated patch v2
> 
> This patch switch the right (tabs) and left (controls) sidebars of Fennec. I
> would like to heard about RTL's people if we want to switch the sidebars or if
> it is better to keep them as if?

Agreed with Khaled here.

> >diff --git a/chrome/content/tabs.xml b/chrome/content/tabs.xml
> >--- a/chrome/content/tabs.xml
> >+++ b/chrome/content/tabs.xml
> >@@ -121,20 +121,28 @@
> >           ]]>
> >         </body>
> >       </method>
> > 
> >       <field name="_columnsCount">1</field>
> >       <method name="_updateWidth">
> >         <body>
> >           <![CDATA[
> >-            let boundingBox = this.firstChild.getBoundingClientRect();
> >-            let columnsCount = Math.ceil(this.childNodes.length / Math.floor(this.style.height / boundingBox.height));
> >+            let tabRect = document.getElementById('tabs-container').getBoundingClientRect();
> >+            let controlsRect = document.getElementById('browser-controls').getBoundingClientRect();
> >+            let rtl = (tabRect.left > controlsRect.left);
> >+
> 
> Please use the helper to know if we are RTL or not and use this.parentNode if
> possible to avoid using document.getElementById("tabs-container");
> Also please use " instead of ' :)

Why can't we just get the computed style for the direction property on the documentElement?

> Thanks a lot for moving forward the patch and doing this work. It was kind of
> out of my sigh for a while!

I'm willing to test a try server patch (which runs on the desktop) and give you guys feedback here.
(In reply to comment #39)
> You could probably add a helper function into BrowserUI to know if the UI is
> RTL or not (see my patch version 3 or 4 I think)
> 
> Please use the helper to know if we are RTL or not and use this.parentNode if
> possible to avoid using document.getElementById("tabs-container");
> Also please use " instead of ' :)

(In reply to comment #40)
> Why can't we just get the computed style for the direction property on the
> documentElement?

Yes I will add a helper function, will clean the code a bit. I guess it should return the direction of the "controls-scrollbox" element or document (can the interface be mixed ltr/rtl?).
(In reply to comment #42)
> Yes I will add a helper function, will clean the code a bit. I guess it should
> return the direction of the "controls-scrollbox" element or document (can the
> interface be mixed ltr/rtl?).

Yes.  For example, if we choose to display URLs in a piece of UI, that part should be LTR.  It's usually best to get the computed direction style of the element you're dealing with.
Remove the ltr/rtl condition in tabs _updateWidth and just use Math.max.

Instead of adding a helper function to check for rtl, just calculate what is really wanted there.
Attachment #465474 - Attachment is obsolete: true
Attachment #467872 - Flags: review?(mbrubeck)
Attachment #399732 - Attachment is obsolete: true
Attachment #467872 - Flags: review?(mbrubeck) → review+
http://hg.mozilla.org/mobile-browser/rev/8e1a796c8f49
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Flags: in-litmus?
Summary: Fennec must support rtl → [RTL] Fennec must support rtl
Note: for the most part RTL is working in arabic if the plugin is used.  setting to verified : 

verified FIXED on build:
Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b7pre)Gecko/20100916 Firefox/4.0b7pre Fennec/2.0b1pre

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100916 Firefox/Fennec/2.0b1pre
Status: RESOLVED → VERIFIED
Assignee: jap → nhirata.bugzilla
Assignee: nhirata.bugzilla → jap
Flags: in-litmus? → in-litmus?(nhirata.bugzilla)
Need more tests, but will create them as I go along with more experimental testing.
Flags: in-litmus?(nhirata.bugzilla) → in-litmus+
You need to log in before you can comment on or make changes to this bug.