180628 - page setup dialog takes RTL where it shouldn't

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Tsahi Asher]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.1) Gecko/20020826
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.1) Gecko/20020826

when using a RTL language pack, the page setup dialog, like most of the rest of
the interface, completely takes RTL: in the Margins box, the box for left margin
input is on the right, and for the right margin on the left. but the data
entered on the right box, titled "left", indeed affects the left margin. so
these two boxes need to stay on their original position even when using a RTL
language pack.

the same goes for the header and footer box at the buttom: the left headers are
shown on the right side, and the right headers are shown on the left side.

Reproducible: Always

Steps to Reproduce:
1. aligning the interface to the right: add these lines to the file intl.css, in
the locale\en-US\global, in the en-US.jar file (the language pack file, in the
chrome folder):

window,dialog,wizard,page {
  direction: rtl;
}

menu { direction: rtl; }

outliner { direction: rtl; }

2. start mozilla
3. open File|Page Setup

Actual Results:  
the dialog fully takes BiDi

Expected Results:  
some elements, as described, should not take BiDi.

it is possible that this can be fixed, at least temporarily, with proper CSS
declarations in the said intl.css file.

screenshot coming up.

Comment 1

[Tsahi Asher]

Attachment: screenshot of page setup dialog in hebrew

Comment 2

[Simon Montagu :smontagu]

Marking dependency on bug 139337. "right" and "left" are really the wrong texts
to have in this dialogue in the first place.

Comment 3

[Shoshannah Forbes]

CONFIRMING with mozilla 1.5 on windows 2003. Also, note that the shading of the
db is wrong (see the right border)

Comment 4

[Shoshannah Forbes]

Attachment: see the incurrect shading of this db in windows 2003

Comment 5

[Tsahi Asher]

if you mean the right side of the right tab, then you are right. it's also there
in WinXP. could be a theme issue though.

not that in the tabs in the Page Info dialog a similar problem exists on the
left side, and in the bookmark properties dialog, it doesn't exist at all.

Comment 6

[Reuven Gonen]

This is a fix in the xul file that forces this box to remain LTR, even when the
interface is RTL

Comment 7

[Reuven Gonen]

This is a fix in the xul file that forces this box to remain LTR, even when the
interface is RTL. i left out part of the xul file that is unmodified.

Comment 8

[Reuven Gonen]

Attachment: the fixed xul file

Comment 9

[Reuven Gonen]

Attachment: screen shot of page setup window

Comment 10

[Tsahi Asher]

could someone please make a decent patch out of attachment 147042 [details] [diff] [review] and review it?

Comment 11

[timeless]

Attachment: per 147042

Comment 12

[Tsahi Asher]

thanks!

Comment 13

[Asaf Romano (gone)]

Comment on attachment 148608 [details] [diff] [review]
per 147042

I'm moving review request to Simon, but i don't know if it's up-to-date.

Comment 14

[Asaf Romano (gone)]

Simon?

Comment 15

[Asaf Romano (gone)]

Comment on attachment 148608 [details] [diff] [review]
per 147042

I will update this patch for xpfe/toolkit

Comment 16

[Simon Montagu :smontagu]

(In reply to comment #15)
> (From update of attachment 148608 [details] [diff] [review])
> I will update this patch for xpfe/toolkit

As I've said before, your time would be better spent fixing bug 139337.

Comment 17

[Asaf Romano (gone)]

(In reply to comment #16)
> (In reply to comment #15)
> > (From update of attachment 148608 [details] [diff] [review])
> > I will update this patch for xpfe/toolkit
> As I've said before, your time would be better spent fixing bug 139337.

It will be nice to have, but _for now_ we should just fix this bug (so you'll 
get the same app from installing lang-pack / Localized App).

Comment 18

[Reuven Gonen]

(In reply to comment #16)
> (In reply to comment #15)
> > (From update of attachment 148608 [details] [diff] [review])
> > I will update this patch for xpfe/toolkit
> 
> As I've said before, your time would be better spent fixing bug 139337.

This is not only a matter of the laft and right lables. the boxes themselves are
opposite which means, that if the interface is RTL and i add a margin of "50" to
the box at the right, the page will actually have a margin of "50" on the left
side of the page. in this case the page preview is also wrong since is shows the
margin of the opposite side (view screenshot). in the case of the screenshot,
the page will have a margin of "50" on the left side and margin of "12.7" on the
right side (the lables are actually right in this case).

Comment 19

[Reuven Gonen]

Attachment: Localized page setup page

Comment 20

[Asaf Romano (gone)]

Attachment: toolkit patch

Comment 21

[Benjamin Smedberg]

Comment on attachment 163458 [details] [diff] [review]
toolkit patch

I am not an appropriate reviewer for this code.

Comment 22

[Simon Montagu :smontagu]

Comment on attachment 163458 [details] [diff] [review]
toolkit patch

I am OK with this solution on the branch, so that the UI is at least consistent
with the results.

Comment 23

[Mike Connor [:mconnor]]

Comment on attachment 163458 [details] [diff] [review]
toolkit patch

This is pretty safe, and a good fix to take for l10n.

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 163458 [details] [diff] [review]
toolkit patch

time is short so if this is going to land it needs to happen asap. a=asa for
aviary checkin.

Comment 25

[Asaf Romano (gone)]

Steffen, I'm sorry to nag you too much lately ;)

Comment 26

[Steffen Wilberg]

Toolkit patch checked into aviary branch and trunk.
Leaving open for xpfe.

Comment 27

[Asaf Romano (gone)]

Still wondering if we want to land the workaround for xpfe as well, or leave it
for bug139337. Simon / Tsahi ?

Comment 28

[Tsahi Asher]

i opened this bug in order to flip the "left" and "right" labels and text fields
in the page setup dialog. from the discussion in bug 139337, it looks like Simon
is planning to do it there. as long as it is done somewhere, i don't really
care, as long as it is done. i don't want it to be forgotten.

Comment 29

[Asaf Romano (gone)]

leaving xpfe patch for bug 139337

Comment 30

[Reuven Gonen]

Attachment: rtl page setup window

this bug got partly broken frome the fix for bug 281545
https://bugzilla.mozilla.org/attachment.cgi?id=177369&action=diff

Comment 31

[Reuven Gonen]

Attachment: set the bottom rows to ltr

I've set the bottom 3 rows to ltr, in order to override the rtl interface

Comment 32

[Asaf Romano (gone)]

Checking in printPageSetup.xul;
/cvsroot/mozilla/toolkit/components/printing/content/printPageSetup.xul,v  <-- 
printPageSetup.xul
new revision: 1.8; previous revision: 1.7
done

Comment 33

[Tsahi Asher]

hey, what about xpfe? it looks to me it will take the better part of a year, if
not more, until SeaMonkey reaches 2.0, when they plan to convert to toolkit.

Comment 34

[(no longer active)]

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