281361 - [RTL UI] tab navigation with left/right arrow keys moves in the wrong direction

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[Asaf Romano (gone)]

Starting from Gecko/20040722 builds (bug 175893), XUL <tab>s are focusable.
However, this functionality is broken in a RTL UI (Hebrew, Arabic, etc.).

STR:
1. aligning the interface to the right: add these rule to userChrome.css

window {
  direction: rtl;
}

2. Open 3 tabs
3. focus (double click) the second tab
4. Click the right arrow key.

ACTUAL RESULTS: last tab is focused.
EXPECTED RESULTS: first tab should be focused

same goes for the left arrow key, in reverse.

Comment 1

[Asaf Romano (gone)]

s/these/this even

Comment 2

[Asaf Romano (gone)]

Attachment: patch v1 (xpfe)

Comment 3

[Simon Montagu :smontagu]

Can you test whether there is a visible impact in performance if you get the
computed style on the keypress instead of caching it?

Comment 4

[[e-mail]]

Comment on attachment 173615 [details] [diff] [review]
patch v1 (xpfe)

>+        document.defaultView.getComputedStyle(this.tabbox, "").getPropertyValue("direction");
You can't be sure of having a tabbox (certainly not without writing code to
implement the getter!), I'd just get the computed style of the tabs themselves.
Also, in chrome, you're allowed to use window.getComputedStyle because we don't
have to be portable. And if [bz/dbaron say that] getComputedStyle is cheap
enough then I don't mind if you make this a property on the tab or inline it
into the handler, but you wouldn't make it a property here.

Comment 5

[Asaf Romano (gone)]

bz	Anyway, the style context is cached
bz	The work done to serialize the style context data into CSS is not cached

seems to be cheap enough

Comment 6

[Asaf Romano (gone)]

Attachment: v1.1 (xpfe/)

Comment 7

[[e-mail]]

Comment on attachment 174585 [details] [diff] [review]
v1.1 (xpfe/)

It would surprise me if people used the arrow keys to switch tabs much anyway.

Comment 8

[Simon Montagu :smontagu]

Comment on attachment 174585 [details] [diff] [review]
v1.1 (xpfe/)

Yeah, I think this is the Right Way to do this. r=me.

Comment 9

[Asaf Romano (gone)]

Comment on attachment 174585 [details] [diff] [review]
v1.1 (xpfe/)

Checking in tabbox.xml;
/cvsroot/mozilla/xpfe/global/resources/content/bindings/tabbox.xml,v  <-- 
tabbox.xml
new revision: 1.34; previous revision: 1.33
done

leaving open for toolkit

Comment 10

[Asaf Romano (gone)]

Attachment: for toolkit/

Comment 11

[Asaf Romano (gone)]

Checking in toolkit/content/widgets/tabbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbox.xml,v  <--  tabbox.xml
new revision: 1.19; previous revision: 1.18
done

Comment 12

[(no longer active)]

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