page setup dialog takes RTL where it shouldn't

RESOLVED FIXED

Status

SeaMonkey
UI Design
RESOLVED FIXED
15 years ago
9 years ago

People

(Reporter: Tsahi Asher, Unassigned)

Tracking

(Blocks: 1 bug, {fixed-aviary1.0, rtl})

Trunk
fixed-aviary1.0, rtl
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(9 attachments)

(Reporter)

Description

15 years ago
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.
(Reporter)

Comment 1

15 years ago
Created attachment 106598 [details]
screenshot of page setup dialog in hebrew
Marking dependency on bug 139337. "right" and "left" are really the wrong texts
to have in this dialogue in the first place.
Depends on: 139337

Comment 3

14 years ago
CONFIRMING with mozilla 1.5 on windows 2003. Also, note that the shading of the
db is wrong (see the right border)
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 4

14 years ago
Created attachment 134691 [details]
see the incurrect shading of this db in windows 2003
(Reporter)

Comment 5

14 years ago
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

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

Comment 7

14 years ago
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

14 years ago
Created attachment 147042 [details] [diff] [review]
the fixed xul file

Comment 9

14 years ago
Created attachment 147043 [details]
screen shot of page setup window
(Reporter)

Comment 10

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

Comment 11

14 years ago
Created attachment 148608 [details] [diff] [review]
per 147042

Updated

14 years ago
Attachment #148608 - Flags: review?(neil.parkwaycc.co.uk)
(Reporter)

Comment 12

14 years ago
thanks!
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.
Attachment #148608 - Flags: review?(neil.parkwaycc.co.uk) → review?(smontagu)
OS: Windows XP → All
Hardware: PC → All
Simon?
Comment on attachment 148608 [details] [diff] [review]
per 147042

I will update this patch for xpfe/toolkit
Attachment #148608 - Flags: review?(smontagu)
Assignee: mkaply → bugs.mano
(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.
(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

13 years ago
(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

13 years ago
Created attachment 160251 [details]
Localized page setup page

Updated

13 years ago
Attachment #160251 - Attachment description: Localized print preview page → Localized page setup page
Created attachment 163458 [details] [diff] [review]
toolkit patch
Attachment #163458 - Flags: review?(bsmedberg)
Attachment #163458 - Flags: approval-aviary?

Comment 21

13 years ago
Comment on attachment 163458 [details] [diff] [review]
toolkit patch

I am not an appropriate reviewer for this code.
Attachment #163458 - Flags: review?(bsmedberg) → review?(bugs)
Flags: blocking-aviary1.0-L10N?
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 on attachment 163458 [details] [diff] [review]
toolkit patch

This is pretty safe, and a good fix to take for l10n.
Attachment #163458 - Flags: review?(bugs) → review+
Flags: blocking-aviary1.0-L10N? → blocking-aviary1.0?
Whiteboard: [have patch]
Whiteboard: [have patch] → [have patch] needs approval

Comment 24

13 years ago
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.
Attachment #163458 - Flags: approval-aviary? → approval-aviary+
Steffen, I'm sorry to nag you too much lately ;)
Whiteboard: [have patch] needs approval → ready to land
Flags: blocking-aviary1.0?
Keywords: fixed-aviary1.0

Comment 26

13 years ago
Toolkit patch checked into aviary branch and trunk.
Leaving open for xpfe.
Whiteboard: ready to land
Still wondering if we want to land the workaround for xpfe as well, or leave it
for bug139337. Simon / Tsahi ?
Component: Layout: BiDi Hebrew & Arabic → XP Apps: GUI Features
Product: Core → Mozilla Application Suite
(Reporter)

Comment 28

13 years ago
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.
leaving xpfe patch for bug 139337
Assignee: bugs.mano → guifeatures
QA Contact: zach

Comment 30

13 years ago
Created attachment 177981 [details]
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

13 years ago
Created attachment 178102 [details] [diff] [review]
set the bottom rows to ltr

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

Updated

13 years ago
Attachment #178102 - Flags: review?(mconnor)

Updated

13 years ago
Attachment #178102 - Flags: review?(mconnor) → review+
Checking in printPageSetup.xul;
/cvsroot/mozilla/toolkit/components/printing/content/printPageSetup.xul,v  <-- 
printPageSetup.xul
new revision: 1.8; previous revision: 1.7
done

Updated

12 years ago
Blocks: 306980

Updated

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Reporter)

Comment 33

12 years ago
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

10 years ago
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl

Updated

9 years ago
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.