Bug 221824 (rtl-themes)

themes should be RTL compatible

NEW
Unassigned

Status

defect
P3
normal
16 years ago
2 years ago

People

(Reporter: tsahi_75, Unassigned)

Tracking

(Blocks 2 bugs, {meta, rtl})

Dependency tree / graph
Bug Flags:
blocking-aviary1.0 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(20 attachments, 9 obsolete attachments)

754 bytes, patch
mconnor
: review+
Details | Diff | Splinter Review
3.40 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
51.25 KB, application/zip
Details
867 bytes, text/plain
Details
10.82 KB, application/zip
Details
526 bytes, application/vnd.mozilla.xul+xml
Details
43.27 KB, image/png
Details
71.53 KB, patch
mano
: review+
Details | Diff | Splinter Review
1.41 KB, patch
steffen.wilberg
: review+
Details | Diff | Splinter Review
1.96 KB, patch
kevin
: review+
Details | Diff | Splinter Review
2.53 KB, patch
mano
: review+
Details | Diff | Splinter Review
4.15 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
12.99 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
952 bytes, patch
Details | Diff | Splinter Review
51 bytes, image/gif
Details
1.30 KB, patch
mano
: review+
Details | Diff | Splinter Review
684 bytes, patch
mano
: review+
Details | Diff | Splinter Review
66.45 KB, patch
Details | Diff | Splinter Review
18.01 KB, application/zip
Details
170.59 KB, application/x-zip-compressed
Details
Reporter

Description

16 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.4) Gecko/20030624

currently, when a language pack of a RTL language (e.g. hebrew, arabic) is used,
the theme graphics remain as they were with a LTR interface. it looks weird with
the Modern and Classic themes, where the back and forward bottons of Navigator
are facing each other instead of opposite sides, and breaks most 3rd party themes.

the problem is mostly with graphics that include arrows: back/foreard, personal
toolbar shevron, sidebar handle, site navigation bar last/next/previous/first,
menu>sub menu arrow, mail: reply, reply all, forward, and others, some of which
depend on the theme.

what i'm suggesting is the following:
1. all images that might be affected by a RTL language pack should have two
sets, one for LTR and one for RTL interface. for some of the images this means
that the RTL Back button, for instance, will simply be the LTR Forward button.
for images that don't have an opposite image, the theme creator will have to
make one.
2. themes will have to be sensitive to the current direction of the interface in
some way.
3. the theme will select the appropriate image, if applicable, depending on the
interface direction. if the image is not affected by the direction (e.g. the
stop button) then the same image is used for both directions.


Reproducible: Always

Steps to Reproduce:
here's how to make the interface RTL, to give you a sence:
1. 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):

/*make UI RTL */

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

menu { direction: rtl; }

outliner { direction: rtl; }

/*
 * make sure search from address bar remains in RTL
 */

#urlbar .autocomplete-search-engine
{
direction: rtl !important;
}

/*
 * keep Composer <HTML> Source tab LTR
 */

#content-source,
#doctype-text { direction: ltr; }

2. start mozilla
Actual Results:  
browser is cross-eyed

Expected Results:  
back button should point right and not left, forward button should point left
and not right, and the same for other buttons i mentioned, and some that i
didn't mention.


it is also possible that in some places, a different CSS will have to be used.
in Modern theme, the extentions buttons (like the back and forward history, or
the two options under the print button) are to the right of their originating
buttons, while they should be to the left in a RTL locale.

Comment 1

16 years ago
i filed a bug a long time ago talking about a selector for this, see if you can
turn it up.
Reporter

Comment 2

16 years ago
i tried, but couldn't find any.
How is chrome supposed to know that it's "rtl" if all that means is that some
CSS rules got set somewhere?
Reporter

Comment 4

16 years ago
don't ask me, i don't know any css. i'm just pointing to a problem, which limits
the number of available themes for RTL locales.

maybe this, from bug 147232 comment 16, will help:

BrowserToolbar[dir=rtl] ForwardButton 
  {
  background: url(RightArrow.png) !important
  }

> 2. themes will have to be sensitive to the current direction of the interface in
> some way.

Unless we have a better way of defining the "current direction of the interface"
than the current ad-hoc setting of "direction" on some random subset of boxes,
that isn't really feasible.

Reporter

Comment 6

16 years ago
i can't say anything smart about this since i don't know javascript or css. it
is possible that the theme mechanism will have to be altered to support this.
Reporter

Comment 7

16 years ago
how about this idea, without knowing much about how things work: the language
pack will have a flag somewhere in one of the dtd files that will tell mozilla
if it's a RTL or a LTR language. that will trigger both the css we currently put
in intl.css in the language pack, and proper behaviour of the theme (maybe like
what is suggested in bug 147232 comment 27 by dbaron. this solution, if
feasable, will revoke the need to what we put in intl.css. it will reside
somewhere else, maybe in the code itself.
Reporter

Comment 8

16 years ago
it is not enough to have flipped images: in Modern theme, the dropmarkers (down
arrows in drop-down menues) are designed with css. a RTL theme will have to use
different css to style them. the same goes for some other elements in the
sidebar and elsewhere, that need a different style rule for a RTL UI.

Updated

15 years ago
Assignee: shliang → themes
OS: Windows XP → All
Hardware: PC → All
Reporter

Comment 9

15 years ago
btw, my suggestion in comment 7 above is already implemented for the <html>
Source tab in composer.

Comment 10

15 years ago
OK, I'd like to push through the idea in comment #7 for firefox 1.0. Having a
DTD flag in global/locale which can be used as a CSS selector is a good idea,
and it will prevent RTL locales from having to ship an entirely different theme
from everyone else.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-aviary1.0?

Comment 11

15 years ago
This is just a locale hook so that the theme can hook into a reliable entity.

Comment 12

15 years ago
Tsahi, I'm interested in knowing why you need these rules: shouldn't they
naturally inherit from the window/page/whatever? Is there some reason why the
.autocomplete-search-engine needs the !important?

menu { direction: rtl; }
outliner { direction: rtl; }
#urlbar .autocomplete-search-engine
{
  direction: rtl !important;
}

Updated

15 years ago
Attachment #160352 - Flags: review?(mconnor)

Comment 13

15 years ago
Are we laying the groundwork for RTL styles to be in the default theme, and the
image flipping stuff will come later? If we're not going to have image flipping,
we'll have to include separate RTL icons.

I can think of more theme icons that are not RTL friendly: Perhaps the bookmark
sidebar has the spine of the book on the wrong side for RTL. Pinstripe's
download sidebar icon also has a little progress meter in the icon which is
going the wrong way for RTL. Also, it seems to me that to do this right we
should redo more of the icons to put the dominant element on the right and the
secondary element(s) on the left ... for instance the new tab and new window
icons should have the + badge on the left.
Comment on attachment 160352 [details] [diff] [review]
Add ltr/rtl as a locale entity for theme hookup

ah, good idea.
Attachment #160352 - Flags: review?(mconnor) → review+

Updated

15 years ago
Attachment #160352 - Flags: approval-aviary?

Comment 15

15 years ago
(In reply to comment #12)
> Tsahi, I'm interested in knowing why you need these rules: shouldn't they
> naturally inherit from the window/page/whatever? 

No. The page directionality should not affect the application directionality.

Simple (and common) scnario:
I have two tabs open- one with a RTL page and the other with LTR page.
What direction should mozilla be? Should it one why for one tab and another way
for another?

Mozilla does that for the scroll bar at some sites:
http://news.nana.co.il/Article/?ArticleID=142213&sid=16

(open the link in a new tab and try to scroll)

Very bad IMO, as I need to move the mouse to the other side of the screen to
scroll (once I notice that the scroll bar is suddenly at the other side, and
that takes time as well) and then when I switch to a LTR tab I have to move the
mouse all the way over again.

Very bad for usability IMO- web pages should not affect the browser UI at all.

Comment 16

15 years ago
Tsahi, perhaps my question was poorly worded, it had nothing to do with
web-pages: since all of our <xul:menu> elements are contained within an
<xul:window>, why do we need a special CSS rule "menu { direction: rtl; }",
since that should, according to the CSS cascade, inherit properly from "window {
direction: rtl; }"
Reporter

Comment 17

15 years ago
(In reply to comment #12)
> Tsahi, I'm interested in knowing why you need these rules: shouldn't they
> naturally inherit from the window/page/whatever? Is there some reason why the
> .autocomplete-search-engine needs the !important?
> 
> menu { direction: rtl; }
> outliner { direction: rtl; }
> #urlbar .autocomplete-search-engine
> {
>   direction: rtl !important;
> }

as i know relatively little CSS, and even less of the chrome's CSS classes,
smontagu told me what CSS to put in the langpack in order to 'flip' it, when i
started with it about two years ago. i was told that outliner doesn't exist anymore.

as for the !important thing, currently the URL bar gets LTR from xul.css (IIRC)
with a class introduced in bug 157607. we add this rule to the langpack to force
RTL in the autocomplete menu and the "search for X at Y" bar.

(In reply to comment #13)
> Are we laying the groundwork for RTL styles to be in the default theme, and the
> image flipping stuff will come later? If we're not going to have image flipping,
> we'll have to include separate RTL icons.
> 

that is correct, Kevin. i once opened a bug for flipping images functionality,
thinking that it would make our lives easier in making RTL langpack. however, i
have since realised that this is not so. images often have shading, which goes
with the shading of the widgets styled with CSS. simply flipping the Back and
Forward buttons, for example, will break that shading. unless, of course,
bsmedberg will find a way to also flip all the shading effects too :) (i am not
sure this is something we want though. MS is doing that in it's localized
products, but if the OS is not localized, it has the shading coming from upper
left, which results in a complete mess in that regard.)

so as i see it now, some of the images will have to have two copies, one for
each UI direction. in other images, we can reuse them in the RTL locale, e.g.
the LTR Back button can be used as the RTL Forward botton.

(In reply to comment #16)
> Tsahi, perhaps my question was poorly worded, it had nothing to do with
> web-pages: since all of our <xul:menu> elements are contained within an
> <xul:window>, why do we need a special CSS rule "menu { direction: rtl; }",
> since that should, according to the CSS cascade, inherit properly from "window {
> direction: rtl; }"

you could be right. like i wrote above, i know very little about this stuff, and
i never tried to play with these rules.

Comment 18

15 years ago
Posted patch Fix submenu arrow (obsolete) — Splinter Review
kmgerich, in combination with the patch above this should fix the arrow
direction in submenus: it requires three images. I did winstripe only, but I
suspect the same patch can be applied verbatim to pinstripe/gnomestripe

Comment 19

15 years ago
Comment on attachment 160491 [details] [diff] [review]
Fix submenu arrow

mconnor, I chose a new "chromedir" attribute instead of "dir" because "dir" has
special-case bidi code and I didn't want to introduce regressions. This patch
shouldn't affect anything in ltr locales, it's just an unused attribute.
Attachment #160491 - Flags: superreview?(webmail)
Attachment #160491 - Flags: review?(mconnor)
Reporter

Comment 20

15 years ago
Reuven, this bug might save you some work.

Comment 21

15 years ago
Comment on attachment 160491 [details] [diff] [review]
Fix submenu arrow

s/%globalRegion;/%globalRegionDTD;/ to make this actually work :(
Comment on attachment 160491 [details] [diff] [review]
Fix submenu arrow

ooh, snazzy, I like
Attachment #160491 - Flags: review?(mconnor) → review+

Comment 23

15 years ago
Comment on attachment 160491 [details] [diff] [review]
Fix submenu arrow

looks good. I'll do a patch for Pinstripe tomorrow
Attachment #160491 - Flags: superreview?(webmail) → superreview+
Comment on attachment 160352 [details] [diff] [review]
Add ltr/rtl as a locale entity for theme hookup

a=asa.
Attachment #160352 - Flags: approval-aviary? → approval-aviary+

Updated

15 years ago
Flags: blocking-aviary1.0? → blocking-aviary1.0+

Comment 25

15 years ago
The entity (attachment 160352 [details] [diff] [review]) has been checked in, branch and trunk.
Keywords: late-l10n

Comment 26

15 years ago
Assignee: themes → bsmedberg
Attachment #160491 - Attachment is obsolete: true
Status: NEW → ASSIGNED

Comment 27

15 years ago
Comment on attachment 160767 [details] [diff] [review]
just the content hook attributes (added back/forward buttons), no actual theme changes

Carrying over mconnor's review, seeking approvals. I will go ahead and land
this on trunk.
Attachment #160767 - Flags: review+

Updated

15 years ago
Attachment #160767 - Flags: approval-aviary?
Reuven says _partly_ flips the livemark icons, so we need his png(s).
Attachment #160767 - Attachment is obsolete: true

Comment 29

15 years ago
i'm attaching the modified files for the rtl theme. the images i flipped are
the 3 files in the browser directory, the chevron icon under global->toolbar,
and the arrows under global->menu. the files in the global->arrow i just
changed the names from "rit" to "lft" and vice versa.

the changes in the css are mainly adjusting the margin, for example in
downloads.css i changed from:

#saveToFolder .toolbarbutton-text {
  text-align: left;
  margin-left: 5px !important;
}

to:

#saveToFolder .toolbarbutton-text {
  text-align: right;
  margin-right: 5px !important;
}

Updated

15 years ago
Attachment #161056 - Attachment filename: rtl theme files.zip → rtl-theme-files.zip
Flags: blocking-aviary1.0+ → blocking-aviary1.0-

Comment 30

15 years ago
Reuven/Mano: right now all I want is a list of images that need to be flipped
for the default theme(s). Kevin Gerich will actually be landing the flipped images.

Please attach it as a text file. Then I can look at the best way to get these
images flipped with the most efficient CSS rules, I am not sure that attachment
160917 [details] [diff] [review] uses the best placement of the new attributes.

Comment 31

15 years ago
The arrows under /browser/arrow/ don't actually need to be flipped. the
filenames can be changed from "lft" to "rit" and vice versa
Reporter

Comment 32

15 years ago
about the shading: i think we should keep the LTR shading (i.e. light comes from
upper left) in the RTL case too, for two reasons:
1. changing the light source to upper-right will break the flow of the OS, which
is in most cases LTR. i don't have localized Windows (or KDE/Gnome), but in the
hebrew localized MS-Office XP, they kept the light source at the upper-left.
also, WinXP has this 3D shading effect for menues and the mouse pointer, so if
we will have the shade on the left, we will end up having shade at both sides of
the menues.
2. changing the shading direction means most images will have to have a RTL
version, to match the new light source, where as now, some of the images can be
used in both cases, sometimes changing position with their mirror counterparts
(mostly arrows that have versions for right and left pointing).

Reuven raised some concerns to me, that the tabs are not well bordered in RTL if
the shading is like in LTR, because they are aligned to the right (the last tab
is the left most tab). there could be other problems like this, so these cases
should be noted.

Comment 34

15 years ago
Comment on attachment 160917 [details] [diff] [review]
[checked in branch] all (i hope) the content hook attributes (except of menu.xml)

Let's get this in.
Attachment #160917 - Flags: review?(bsmedberg)
Attachment #160917 - Flags: review+
Attachment #160917 - Flags: approval-aviary+

Updated

15 years ago
Alias: rtl-themes
Attachment #160917 - Attachment description: all (i hope) the content hook attributes (except of menu.xml) → [checked in branch] all (i hope) the content hook attributes (except of menu.xml)

Comment 35

15 years ago
I also checked in the content hook for menu.xml from attachment 160491 [details] [diff] [review]
This bug seems to have an aviary branch checkin associated with it. If this has
landed on the aviary branch (as much as it's going to for 1.0) can you please
add the "fixed-aviary1.0" keyword? Thanks.

Comment 37

15 years ago
OK. This is not all-the-way fixed; we're for images checkin by kmgerich, but I
have added the keyword for the benefit of some queries.
Keywords: fixed-aviary1.0

Comment 38

15 years ago
Posted file RTL Image Files
RTL images. will check in shortly
Comment on attachment 163093 [details]
RTL Image Files

Thank you Kevin.

Does it include all the files Reuven mentioned?

Comment 40

15 years ago
These are the images that were included:

Go-rtl.png
Menu-arrow-hover-rtl.png
sbtab-twisty-rtl.gif
chevron-rtl.gif
Menu-arrow-disabled-rtl.png
livemark-item-rtl.png
page-livemarks-rtl.png
Weblink-rtl.png
Menu-arrow-rtl.png

For the others, the arrows are already in the theme, you just need a different
css rule.

I didn't include the images on Mac. I will soon.
bsmedberg/kevin, what about the css rules?

Comment 42

15 years ago
I had assumed the css would be in the language pack jar .. I can do the CSS if
you need it, but I probably won't get to it for another couple of days.
Reporter

Comment 43

15 years ago
(In reply to comment #42)
> I had assumed the css would be in the language pack jar .. I can do the CSS if
> you need it, but I probably won't get to it for another couple of days.

no, they will be in the themes shiped by mozilla.org. something like attachment
160491 [details] [diff] [review].
(In reply to comment #42)
> I had assumed the css would be in the language pack jar .. I can do the CSS if
> you need it, 

This is not an option as we can't apply style rules for winstripe/pinstripe only.

> but I probably won't get to it for another couple of days.

as long as it is before One-Dot-Oh final release, t's more than enough. :)

Comment 45

15 years ago
Posted image RTL styles in action (obsolete) —
Patch coming shortly.
Kevin, i forgot to say: we need to replace
1) every margin-right to -moz-margin-end (which is "right" in the rtl case and
"left" in the rtl case)
2) every margin-left to -moz-margin-start
3) every padding-left to -moz-padding-start
4) every padding-right to -moz-margin-end 
^^ That's in order to make rtl themes look as expected).

Comment 48

15 years ago
Here it is. I had to add the global region DTD to the help window because it
wouldn't have known about chromedir otherwise. Also in menu.xml the chromedir
was missing from the menu-iconic binding, so I added that.

Comment 49

15 years ago
Comment on attachment 163998 [details] [diff] [review]
RTL style patch .. CSS and additions to menu.xml, help.xul

Requesting review and superreview. Is Mike still gone?
Attachment #163998 - Flags: superreview?(mconnor)
Attachment #163998 - Flags: review?(bsmedberg)

Comment 50

15 years ago
Asaf, do you have time to come up with a patch for the padding and margins?
(In reply to comment #50)
> Asaf, do you have time to come up with a patch for the padding and margins?

I'm on it.

Comment 52

15 years ago
Also be careful of padding and margins set by shorthand, like

padding: 1px 2px 3px 4px;

would we have to unroll them all into four separate declarations?
(In reply to comment #52)
> Also be careful of padding and margins set by shorthand, like
> 
> padding: 1px 2px 3px 4px;
> 
> would we have to unroll them all into four separate declarations?

yep :( w-i-p.
Actually, not all. just cases where (margin-right != margin-left) :)
Comment on attachment 163998 [details] [diff] [review]
RTL style patch .. CSS and additions to menu.xml, help.xul

>Index: toolkit/themes/winstripe/help/help.css
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/themes/winstripe/help/help.css,v
>retrieving revision 1.1.2.8
>diff -u -p -8 -r1.1.2.8 help.css
>--- toolkit/themes/winstripe/help/help.css	16 Oct 2004 02:05:05 -0000	1.1.2.8
>+++ toolkit/themes/winstripe/help/help.css	30 Oct 2004 18:14:32 -0000
>@@ -70,23 +70,28 @@ menubutton:not([disabled="true"]):hover,
> toolbarbutton:not([disabled="true"]):hover:active,
> menubutton:not([disabled="true"]):hover:active {
> 	color: ButtonText !important;
> }
> 
> /* Set the minimum sidebar width so the help contents aren't squeezed together.*/
> #helpsidebar-box { width: 200px; }
> 
>-#help-back-button { -moz-image-region: rect(0px 24px 24px 0px); }
>-#help-back-button[buttonover="true"] { -moz-image-region: rect(24px 24px 48px 0px); }
>-#help-back-button[disabled="true"] { -moz-image-region: rect(48px 24px 72px 0px); }
>-
>-#help-forward-button { -moz-image-region: rect(0px 48px 24px 24px); }
>-#help-forward-button[buttonover="true"] { -moz-image-region: rect(24px 48px 48px 24px); }
>-#help-forward-button[disabled="true"] { -moz-image-region: rect(48px 48px 72px 24px); }
>+#help-back-button, 
>+#help-forward-button[chromedir="rtl"] { -moz-image-region: rect(0px 24px 24px 0px); }
>+#help-back-button[buttonover="true"], 
>+#help-forward-button[buttonover="true"][chromedir="rtl"] { -moz-image-region: rect(24px 24px 48px 0px); }
>+#help-back-button[disabled="true"], 
>+#help-forward-button[disabled="true"][chromedir="rtl"] { -moz-image-region: rect(48px 24px 72px 0px); }
>+
>+#help-forward-button, #help-back-button[chromedir="rtl"] { -moz-image-region: rect(0px 48px 24px 24px); }
>+#help-forward-button[buttonover="true"], 
>+#help-back-button[buttonover="true"][chromedir="rtl"] { -moz-image-region: rect(24px 48px 48px 24px); }
>+#help-forward-button[disabled="true"], 
>+#help-back-button[disabled="true"][chromedir="rtl"] { -moz-image-region: rect(48px 48px 72px 24px); }

I guess this is prevailing style, but yech.

>-}
>+}
>\ No newline at end of file

newline please :)

r=mconnor@steelgryphon.com

(and yes, this is the exception, not the rule)
Attachment #163998 - Flags: superreview?(mconnor)
Attachment #163998 - Flags: review?(bsmedberg)
Attachment #163998 - Flags: review+
Comment on attachment 164013 [details] [diff] [review]
margin/padding winstripe fix

Kevin, I hope you will find enough time and motivation to read this boring one
:-)

We should get this in really soon, so we will have enough time to test both
rtl/ltr builds.
Attachment #164013 - Flags: review?(webmail)
Accroding to Mac Pinstripe: There is no need to create rtl images / css rules
for it. Altough there are some translated (hebrew / arbic) apps, no one of them
is RTLed.

In order to avoid the mac theme to be fliped, i suggest to add the following css
rules to pinstripe:

window, dialog, page, menu
{
  direction: ltr !important;
}
Reporter

Comment 59

15 years ago
kevin, is it possible to make a RTL version of the Add Tab button? in LTR, the
plus is on the right, because new tabs are added to the right. in RTL, new tabs
are added to the left, so the plus should be on the left side of the image.
Reporter

Comment 61

15 years ago
and another image: the show/hide index pane button in the help window needs a
RTL version, because in RTL the pane is on the right.

Comment 62

15 years ago
There are many more icons that need RTL equivalents (new window, new tab, new
bookmark, bookmark sidebar, etc etc.) but I think this is the best we can do for
now. I want to land this and get on to other things before 1.0.

Of course feel free to submit patches for other issues you come across, but
they'll be considered separately.

On the Mac issue, that's great to hear that I don't have to do this again for
Pinstripe today :) but we should probably have another bug requesting that
Pinstripe be RTL compatible and target it at Firefox 1.1.

Updated

15 years ago
Attachment #163998 - Flags: approval-aviary?

Comment 63

15 years ago
(In reply to comment #56)
> Created an attachment (id=164013)
> margin/padding winstripe fix
> 
> 
> *argh(!)*

a few comments:

1. as a general rule, all the left and right attributes most be changed unless
you know their is a reason why not to (which is a very good idea). 

2. i think we discussed the -moz-margin-start: attribute. do you know for fact
that it's working?

3. they are a few more attributes that need to be changed. these are just
examples, so you need to change them accordingly. also i'm using here the left
values, but you need to change both directions:

text align: left;
border left: none;

in the tabbox.css file:

-moz-appearance: tab-left-edge; , -moz-border-radius-topleft: 0; ,
-moz-border-radius-bottomleft: 0; these are actually pretty significant changes.

i also change the shadows of -moz-border-left-colors, and border-left. i had a
discussion with tsahi about the menus shadows, since windows xp adds a shaddow
to the left side of the menu, so the shaddow appears at both directions. i
personnaly think that it's better to change the shaddow in the menus as well
since i didn't see this problem on other platforms, and also in windows xp i
think it's better to have the shaddow in both sides, instead of just on the
wrong side.

sorry if i'm too meticulous, but after all I am using the Hebrew interface of
firefox, and i want it to look as good as possible :-)
(In reply to comment #63)

> 2. i think we discussed the -moz-margin-start: attribute. do you know for fact
> that it's working?

see last attachment.

> 3. they are a few more attributes that need to be changed. these are just
> examples, so you need to change them accordingly. also i'm using here the left
> values, but you need to change both directions:
> 
> text align: left;
> border left: none;

please mail me a list of all these cases, this won't be automatic as the last
patch...


> i also change the shadows of -moz-border-left-colors, and border-left. i had a
> discussion with tsahi about the menus shadows, since windows xp adds a shaddow
> to the left side of the menu, so the shaddow appears at both directions. i
> personnaly think that it's better to change the shaddow in the menus as well
> since i didn't see this problem on other platforms, and also in windows xp i
> think it's better to have the shaddow in both sides, instead of just on the
> wrong side.

We're not going to (and we shoudn't) change the shaddows.

Comment 66

15 years ago
(In reply to comment #65)
> (In reply to comment #63)
> 

> > 3. they are a few more attributes that need to be changed. these are just
> > examples, so you need to change them accordingly. also i'm using here the left
> > values, but you need to change both directions:
> > 
> > text align: left;
> > border left: none;
> 
> please mail me a list of all these cases, this won't be automatic as the last
> patch...

the list here includes all the attributes i know for now.

> > i also change the shadows of -moz-border-left-colors, and border-left. i had a
> > discussion with tsahi about the menus shadows, since windows xp adds a shaddow
> > to the left side of the menu, so the shaddow appears at both directions. i
> > personnaly think that it's better to change the shaddow in the menus as well
> > since i didn't see this problem on other platforms, and also in windows xp i
> > think it's better to have the shaddow in both sides, instead of just on the
> > wrong side.
> 
> We're not going to (and we shoudn't) change the shaddows.

why not?

Comment on attachment 164013 [details] [diff] [review]
margin/padding winstripe fix

Kevin?
Attachment #164013 - Flags: approval-aviary?

Comment 68

15 years ago
Comment on attachment 164013 [details] [diff] [review]
margin/padding winstripe fix

Please fix the typo otherwise, r=me. This was a great read :)

Index: toolkit/themes/winstripe/global/checkbox.css
===================================================================
RCS file: /cvsroot/mozilla/toolkit/themes/winstripe/global/checkbox.css,v
retrieving revision 1.1.2.1
diff -u -r1.1.2.1 checkbox.css
--- toolkit/themes/winstripe/global/checkbox.css	5 Jun 2004 21:31:31
-0000	    1.1.2.1
+++ toolkit/themes/winstripe/global/checkbox.css	30 Oct 2004 21:45:25
-0000
@@ -33,17 +33,20 @@
   -moz-appearance: checkbox-container;
   -moz-box-align: center;
   margin: 2px 4px;
-  padding: 1px 2px 1px 4px;
+  padding-top: 1px;
+  padding-bottom: 1px;
+  -moz-padding-start: 4px;
+  -moz-padding-end: 2p;x
 }
Attachment #164013 - Flags: review?(webmail) → review+
Comment on attachment 164013 [details] [diff] [review]
margin/padding winstripe fix

Any chance to get these two patches before 1.0?

Both are pretty safe (no "real" code changes, only additions for rtl).

It currently blocks the he-IL release rom being offical.
Comment on attachment 164013 [details] [diff] [review]
margin/padding winstripe fix

too late for 1.0
Attachment #164013 - Flags: approval-aviary? → approval-aviary-
Comment on attachment 163998 [details] [diff] [review]
RTL style patch .. CSS and additions to menu.xml, help.xul

too late for 1.0
Attachment #163998 - Flags: approval-aviary? → approval-aviary-
Comment on attachment 164013 [details] [diff] [review]
margin/padding winstripe fix

The margin patch still applies. Kevin, would you be able to check it in?
Posted patch winstripe: all in one (obsolete) — Splinter Review
moving r= forn kmgerich/mconnor.

I'll check it in later today.
Attachment #163998 - Attachment is obsolete: true
Attachment #164013 - Attachment is obsolete: true
Attachment #174134 - Flags: review+
Comment on attachment 174134 [details] [diff] [review]
winstripe: all in one

oops, wrong diff
Attachment #174134 - Attachment is obsolete: true
Attachment #174134 - Flags: review+
checked in 2005-02-20 07:49
Attachment #174135 - Attachment is obsolete: true
Attachment #174854 - Flags: review+
Comment on attachment 174854 [details] [diff] [review]
Winstripe patch: checked in

Part of this regressed bug 257280 Ev1 patch.

Please, check-in a corrective patch,
changing the |#help-forward-button[buttonover="true"][chromedir="rtl"]|-like
syntax to |#help-forward-button[chromedir="rtl"]:hover| in these files.

Thanks.
(In reply to comment #78)
> (From update of attachment 174854 [details] [diff] [review] [edit])
> Part of this regressed bug 257280 Ev1 patch.
> 
> Please, check-in a corrective patch,
> changing the |#help-forward-button[buttonover="true"][chromedir="rtl"]|-like
> syntax to |#help-forward-button[chromedir="rtl"]:hover| in these files.
> 

Apparently, this was only help.css (the other instances werem't fixed in bug
257280).
(In reply to comment #80)
> Apparently, this was only help.css (the other instances werem't fixed in bug
> 257280).

Right: "not yet" let's say ;->

While I agree that you can leave the unmodified ones for me to fix in that other
bug,
it would be nice if you could convert the ones you added too...

Comment 83

14 years ago
Comment on attachment 176046 [details] [diff] [review]
[checked in] winstripe: new pref window fixes

looks good. Thanks Asaf!
Attachment #176046 - Flags: review?(webmail) → review+
Comment on attachment 176046 [details] [diff] [review]
[checked in] winstripe: new pref window fixes

Checking in preferences.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/preferences/preferences.css,v
 <--  preferences.css
new revision: 1.3; previous revision: 1.2
done
Attachment #176046 - Attachment description: winstripe: new pref window fixes → [checked in] winstripe: new pref window fixes

Comment 85

14 years ago
Comment on attachment 175124 [details] [diff] [review]
[checked in] buttonover -> :hover

r=me.
Attachment #175124 - Flags: review?(webmail) → review+
Comment on attachment 176046 [details] [diff] [review]
[checked in] winstripe: new pref window fixes

This file is "broken";
please, fix the Trunk:
{{
201 #ChangeActionDialog {
  padding: 0px;
202 }
203 
#ChangeActionDialog .dialog-button-box {
204   padding: 8px 10px 10px 8px;
}
205 
#changeActionHeader {
206   border-bottom: 2px groove ThreeDFace;
207   margin: 0px;
208   padding: 10px;
209   background-color: -moz-Field;
210   color: -moz-FieldText;  
211 }
212 
#changeActionContent {
213   padding: 8px 10px 10px 9px;
214 }
215 
#defaultAppIcon {
216   width: 16px;
  height: 16px;
217   margin-left: 2px;
}
218 
}}
Comment on attachment 174854 [details] [diff] [review]
Winstripe patch: checked in

'padding:' line should have been removed in the following rules:
(please fix them)


>Index: toolkit/themes/winstripe/global/button.css
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/themes/winstripe/global/button.css,v

>@@ -99,30 +102,37 @@ button[checked="true"] {
> button[checked="true"] > .button-box {
>   padding: 2px 3px 1px 4px;
>+  padding-top: 2px;
>+  padding-bottom: 1px;
>+  -moz-padding-start: 4px;
>+  -moz-padding-end: 3px;
> }


>Index: toolkit/themes/winstripe/global/menulist.css
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/themes/winstripe/global/menulist.css,v

>@@ -121,16 +124,20 @@ menulist[disabled="true"] {
> .menulist-editable-box {
>   padding: 3px 0px 3px 2px;
>+  padding-top: 3px;
>+  padding-bottom: 3px;
>+  -moz-padding-start: 2px;
>+  -moz-padding-end: 0px;
> }


>Index: toolkit/themes/winstripe/global/radio.css
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/themes/winstripe/global/radio.css,v

>@@ -42,27 +42,34 @@
> .radio-label-box {
>-  margin-left: 2px;
>+  -moz-margin-start: 2px;
>   border: 1px solid transparent;
>   padding: 0px 0px 1px 1px;
>+  padding-top: 0px;
>+  padding-bottom: 1px;
>+  -moz-padding-start: 1px;
>+  -moz-padding-end: 0px;
> }


>Index: toolkit/themes/winstripe/global/toolbarbutton.css
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/themes/winstripe/global/toolbarbutton.css,v

>@@ -51,49 +51,56 @@ toolbarbutton {
> toolbarbutton[checked="true"] {
>   border-color: ThreeDShadow ThreeDHighlight ThreeDHighlight ThreeDShadow !important;
>   padding: 4px 2px 2px 4px !important;
>+  padding-top: 4px !important;
>+  padding-bottom: 2px !important;
>+  -moz-padding-start: 4px !important;
>+  -moz-padding-end: 2px !important;
>   background-image: url("chrome://global/skin/toolbar/Lighten.png");
>   color: ButtonText;
> }
(In reply to comment #87/#88)
everything should be fixed now, please verify
(In reply to comment #88)
> (In reply to comment #87/#88)
> everything should be fixed now, please verify

LXR has not updated yet, but Bonsaï looks fine; for the patch and the two comments.
Thanks.

Comment 90

14 years ago
I've notice they are still 2 problems:
1. going to file->print preview, the back and next arrows are switched.
2. the tab shadows are incorrect. to reproduce open a new tab, select it, then
go back and reselect the first one. the end of the next tab seems to be cut.
Attachment #175124 - Attachment description: buttonover -> :hover → [checked in] buttonover -> :hover

Updated

14 years ago
Attachment #177604 - Flags: review?(benjamin) → review+
Comment on attachment 177604 [details] [diff] [review]
[checked in] tabbox fixes (winstripe)

Checking in toolkit/content/widgets/tabbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbox.xml,v  <--  tabbox.xml
new revision: 1.22; previous revision: 1.21
done
Checking in toolkit/themes/winstripe/global/tabbox.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/tabbox.css,v  <--  tabbox.css
new revision: 1.11; previous revision: 1.10
done
Attachment #177604 - Attachment description: tabbox fixes (winstripe) → [checked in] tabbox fixes (winstripe)

Comment 93

14 years ago
the &locale.dir inside tabbox.xml is set to "tabs" instead of "tab"
updating also tabbox.css
Attachment #177604 - Attachment is obsolete: true
Comment on attachment 178105 [details] [diff] [review]
[checked in] fix tabbox.xml and tabbox.css

right, thanks.
Attachment #178105 - Attachment description: fix menu.xml and tabbox.css → fix tabbox.xml and tabbox.css
Attachment #178105 - Flags: review+
Comment on attachment 178105 [details] [diff] [review]
[checked in] fix tabbox.xml and tabbox.css

Checking in toolkit/themes/winstripe/global/tabbox.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/tabbox.css,v  <--  tabbox.css
new revision: 1.12; previous revision: 1.11
done
Checking in toolkit/content/widgets/tabbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbox.xml,v  <--  tabbox.xml
new revision: 1.23; previous revision: 1.22
done
Attachment #178105 - Attachment description: fix tabbox.xml and tabbox.css → [checked in] fix tabbox.xml and tabbox.css

Comment 96

14 years ago
this patch is to fix the right and left arrows that are on the print preview
toolbar.
Attachment #178235 - Flags: review?(benjamin)

Updated

14 years ago
Attachment #178235 - Flags: review?(benjamin) → review+
Comment on attachment 178235 [details] [diff] [review]
[checked in] fix for printPreview toolbar arrows

Checking in toolkit/components/printing/content/printPreviewBindings.xml;
/cvsroot/mozilla/toolkit/components/printing/content/printPreviewBindings.xml,v
 <--  printPreviewBindings.xml
new revision: 1.18; previous revision: 1.17
done
Checking in toolkit/themes/winstripe/global/printPreview.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/printPreview.css,v  <-- 
printPreview.css
new revision: 1.5; previous revision: 1.4
done
Attachment #178235 - Attachment description: fix for printPreview toolbar arrows → [checked in[ fix for printPreview toolbar arrows
Attachment #178235 - Attachment description: [checked in[ fix for printPreview toolbar arrows → [checked in] fix for printPreview toolbar arrows

Updated

14 years ago
Assignee: benjamin → bugs.mano
Status: ASSIGNED → NEW
QA Contact: bugs.mano → benjamin
The rest of this bug (which is mainly classic and modern, and firefox's
pinstripe) aren't really something I'm going to workon -> helpwanted.

If there are more issues with Winstripe, please open new bugs (against me), thanks.
Keywords: helpwanted
Priority: -- → P3
Target Milestone: --- → Future
Reporter

Comment 99

14 years ago
the new seamonkey council announced they plan to move seamonkey to toolkit
(http://wiki.mozilla.org/SeaMonkey:Home_Page#The_plan and
http://wiki.mozilla.org/SeaMonkey:Project_Goals#Future_Releases). in light of
this, is there a point in working on this? will current themes work with a
toolkit seamonkey?

Comment 100

14 years ago
(In reply to comment #99)
> the new seamonkey council announced they plan to move seamonkey to toolkit
> (http://wiki.mozilla.org/SeaMonkey:Home_Page#The_plan and
> http://wiki.mozilla.org/SeaMonkey:Project_Goals#Future_Releases). in light of
> this, is there a point in working on this? will current themes work with a
> toolkit seamonkey?

I think this will add a new theme to the themes directory, and will probably be
something separate (like Firefox and Thunderbird have) once it's in, we can
apply this patch to seamonkey as well.

Comment 101

14 years ago
the chevron-rtl.png icon does not appear in:
http://lxr.mozilla.org/seamonkey/source/toolkit/themes/winstripe/global/toolbar/
which results in no "chevron" icon in the he-IL interface.

Comment 102

14 years ago
This patch adjusts the theme for Linux to be RTL compatible.

Updated

14 years ago
Attachment #184246 - Flags: review?(benjamin)

Updated

14 years ago
Attachment #184246 - Flags: review?(benjamin) → review+
Comment on attachment 184246 [details] [diff] [review]
[checked in] patch for Linux theme

a=shaver
Attachment #184246 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Comment on attachment 184246 [details] [diff] [review]
[checked in] patch for Linux theme

checked in.
Attachment #184246 - Attachment description: patch for Linux theme → [checked in] patch for Linux theme

Comment 105

14 years ago
remember this is nu such thing as "0px;" is just called "0;". could save a few
bytes...:)

Updated

14 years ago
Attachment #187181 - Flags: approval-aviary1.1a2?

Updated

14 years ago
Attachment #187181 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Attachment #187181 - Attachment description: adds chevron-rtl.gif to jar → [checked in] adds chevron-rtl.gif to jar

Comment 108

14 years ago
fix fot the new preferences window changes...
Attachment #187182 - Attachment description: goes in toolkit/themes/winstripe/global/toolbar/chevron-rtl.gif → [checked in] goes in toolkit/themes/winstripe/global/toolbar/chevron-rtl.gif

Comment 109

14 years ago
minor fix for "window.dialog" padding in global.css

Updated

14 years ago
Attachment #187302 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Comment on attachment 187301 [details] [diff] [review]
[checked in] fix for preferences.css

a=bsmedberg for landing today (6/29)
Attachment #187301 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Comment on attachment 187302 [details] [diff] [review]
[checked in] fix padding in global.css

Checking in global.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/global.css,v  <--  global.css
new revision: 1.8; previous revision: 1.7
done
Attachment #187302 - Attachment description: fix padding in global.css → [checked in] fix padding in global.css
Comment on attachment 187301 [details] [diff] [review]
[checked in] fix for preferences.css

Checking in preferences.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/preferences.css,v  <-- 
preferences.css
new revision: 1.3; previous revision: 1.2
done
Attachment #187301 - Attachment description: fix for preferences.css → [checked in] fix for preferences.css

Updated

14 years ago
Blocks: 306980
*** Bug 317050 has been marked as a duplicate of this bug. ***
Reporter

Comment 114

14 years ago
from bug 317050:

Twisty is not aligned properly in search bar

Seen using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; he; rv:1.8)
Gecko/20051117 Firefox/1.5

In the search bar location, the twisty is not aligned properly, causing it to
be inside of the Google icon. Screenshot coming.

-a mac-specific problem.

Comment 115

14 years ago
Posted patch patch for pinstripe (obsolete) — Splinter Review
this is a complete patch for the mac theme.

this is just a draft, since i'm still having a minor problem with the bookmarks(on the personal toolbar) hover/open color, which i haven't solved yet. this problem just effects the RTL interface.

Comment 116

14 years ago
i had some unnecessary lined in the previous patch that i removed in this one.
Attachment #205078 - Attachment is obsolete: true

Comment 117

14 years ago
these images that are needed for this patch. these images are simply the original ones flipped horizontally.
As Firefox 1.5 became official, there are few things left buggy in the default theme. Tsahi said the Windows version looks complete, but for me I see the Linux version with few design bugs.

Comment 119

14 years ago
(In reply to comment #118)
> Created an attachment (id=205174) [edit]
> Screenshots: Things left behind in Firefox 1.5/Linux
> 
> As Firefox 1.5 became official, there are few things left buggy in the default
> theme. Tsahi said the Windows version looks complete, but for me I see the
> Linux version with few design bugs. 
> 

most of the problems in the screenshots are not releated to the screenshots, but to something in the gtk UI. the problem with the reversed arrows is reported in bug 316748 and with the checkmarks and bullets in bug 297790. regarding the tabs issue, this was fixed in windows with patch 178105. i couldn't find a way to change the tab appearence in Linux in the Css, so i guess this belongs to something in the gtk UI as well (my guess is that both ot these problems have the same source). The problem with the Firefox word aligned to the left appears in windows as well. the interesting part about this is that if you open the preferences window the second time it appears correctly.

we should probably open two additional bugs for the tabs and the preferences "Firefox" alignment.

Comment 120

14 years ago
(In reply to comment #119)
> (In reply to comment #118)
> > Created an attachment (id=205174) [edit]
> > Screenshots: Things left behind in Firefox 1.5/Linux
> > 
> > As Firefox 1.5 became official, there are few things left buggy in the default
> > theme. Tsahi said the Windows version looks complete, but for me I see the
> > Linux version with few design bugs. 
> > 
> 
> most of the problems in the screenshots are not releated to the screenshots,
> but to something in the gtk UI.

i meant are not related to the theme :-)

Comment 121

13 years ago
What's going on with this bug? I know Firefox 2 will have all-new themes. Will this bug be fixed? Will it be even worse? Pinstripe is still completely busted, and gnome still has issues. 

One issue I have which I've not seen mentioned here is that in LTR mode, firefox highlights both the active tab as well as the tab twice to the right from the active tab (if it exists). This does not happen when I use the English interface (same build with xpi added and language autodection code enabled). 
Reporter

Comment 122

13 years ago
FF 1.5 already has RTL compatible theme. what remains is having a SeaMonkey RTL compatible theme (originally i opened this bug for the Suite, but that's irrelevant now). as for the theme for FF 2, we should check on that.

Comment 123

13 years ago
It is not true that Firefox has RTL compatible themes. Not everyone uses the expeirence-points (XP) based operating system. As I (and others) have noted, Firefox on the Mac has no RTL compatability, and even the GTK version has some issues left. 

While I'm ranting: it seems that only the default theme is RTL compatible. Other themes are not. Is this intentional? It would be nice if there a way to make themes RTL compatible (or at the very least get Forward/Back right :) without special effort on the part of theme designers, although I guess this may not be possible. 
note to asaf, for my landing for bug I *did not* check in the following changes, but I think you will want to make them to scrollbox.css (for pinstripe and winstripe):


< .autorepeatbutton-up
> .autorepeatbutton-up, .autorepeatbutton-down[chromedir="rtl"]

and

< .autorepeatbutton-down
> .autorepeatbutton-down, .autorepeatbutton-up[chromedir="rtl"]

please let me know if you have questions.
Depends on: tabbedbrowsing
Please do commit this change to winstripe.

Pinstripe (as i noted in the other bug) shouldn't be changed until we fix the whole theme.
> Please do commit this change to winstripe.
>
>Pinstripe (as i noted in the other bug) shouldn't be changed until we fix the
>whole theme.

thanks for clarifying.  I'll go fix this on winstripe, but not pinstripe.
> I'll go fix this on winstripe, but not pinstripe.

winstripe change checked in, and I will check it into the branch once I get approval for the the branch, but the pinstripe change will not be checked in to either trunk or branch.
Assignee: mano → nobody
Keywords: helpwantedmeta
QA Contact: benjamin → themes

Updated

12 years ago
Blocks: 402273

Updated

12 years ago
Blocks: fx3-l10n-he

Updated

11 years ago

Updated

11 years ago
Blocks: fx35-l10n-fa

Updated

11 years ago
No longer blocks: Persian-Fx3.5

Comment 128

11 years ago
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl
Product: Core → SeaMonkey

Updated

10 years ago
No longer blocks: fx35-l10n-fa

Updated

10 years ago
Blocks: Persian

Updated

10 years ago
No longer blocks: Persian-Fx3.5
You need to log in before you can comment on or make changes to this bug.