Closed Bug 140636 Opened 18 years ago Closed 9 years ago

menu items are not aligned on the same line when UI aligned to the right

Categories

(SeaMonkey :: Themes, defect)

x86
Windows 98
defect
Not set

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 311383

People

(Reporter: tsahi_75, Unassigned)

Details

(Keywords: rtl)

Attachments

(4 files)

build ID: 2002041711 (Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0rc1)
Gecko/20020417)

Description: when the interface is aligned to the right, e.g. when using a RTL
language (like hebrew or arabic), not all menu items are aligned on the same
line. the right ends of the menu items are not all one under the other. this
happens on almost all menues of the Navigator main menu.

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 one of the main menues
4. observe the menu items alignment

Actual Results: some items are slightly indented (by 1-3 pixels) compared to
other items.

Expected results: all items should be aligned on the same line, one under the other.
Attached image screenshot
i added a thin gray line along the right end of the menu items, to emphasize
the misalignment.
That looks like a problem with the font -- the "r" and "t" glyphs take up more
space than they actually paint...
i can show this with hebrew characters as well. hebrew characters are much more
"square" than english characters, yet it happens too. do you want a screenshot?
would be nice, yes.  :)
this time i don't even have to add a helper line, the misalignment is obvious.
the first line even starts off the menu box.
Status: UNCONFIRMED → NEW
Ever confirmed: true
forgot to add, this last screenshot is from a moz0.9.8. i don't have a hebrew
language pack for 1.0 yet.
and something else i noticed: in the english locale, there is a gap of about 1cm
between the text and the left edge of the menu box. in hebrew, there is none.
all the strings should start at least 15 pixels to the left of where they are.
this still happenes in mozilla 1.5.
it just occurred to me, that this bug may be related to bug 74880.
Does this bug still occur? I don't see it in mozilla 1.7b (Hebrew system 
locale, untranslated UI).
when i reported this, i was using Win98, which has this square font for hebrew
applications, that mozilla used. now i use WinXP, and mozilla uses a more
subtle, round font. i don't know if that's related, but with mozilla 1.6 (i
don't have a language pack for 1.7 yet) the menu items are mostly aligned on the
same line, but i still have the problem of lack of indentation: the text starts
about 3-4 pixels from the right edge of the menu, instead of 15 or so. only menu
items that have bullets or "v" marks at their begining (or folder images for
bookmarks) are indented, so eventually the text is not all on the same line.

i'd like to know how the latest hebrew versions of mozilla look like on Win98,
if someone can post it here.

in any case, the problem happenes when the hebrew language pack is enabled.
Tsahi, can you replace occurrences of 'margin-left' to '-moz-margin-start', in:

chrome\modern.jar/skin\modern\global\menu.css      or/and
chrome\classic.jar/skin\classic\global\menu.css

(depending on your skin)?

e.g.
.menu-text {
  -moz-margin-start: 18px !important;
  font-weight: inherit;
}

Does it solve the problem of lacking indentation? If so, I'll release a 
proposed patch for this.
nop. i tried it for modern, and it didn't change anything.
Sorry, I forgot it won't work for older releases - only starting from 1.7, I 
guess... Did you try it in mozilla 1.6?
yes. then i'll try it with a 1.7 nightly i have from last week, later today.
(In reply to comment #12)
> Tsahi, can you replace occurrences of 'margin-left' to '-moz-margin-start', in:
> 
> chrome\modern.jar/skin\modern\global\menu.css      or/and
> chrome\classic.jar/skin\classic\global\menu.css
> 
> (depending on your skin)?
> 
> e.g.
> .menu-text {
>   -moz-margin-start: 18px !important;
>   font-weight: inherit;
> }
> 
> Does it solve the problem of lacking indentation? If so, I'll release a 
> proposed patch for this.
> 

yes, this solves it.
please patch both the 1.7 branch and the trunk.
Attached patch patchSplinter Review
Replaces left/right margins and paddings with the -moz-*-start/end shorthand
properties.

Custom themes, supplied with their own css, will be still experiencing the 
lack of indentation.

Simon, can you please review the patch?
Attachment #150366 - Flags: review?(smontagu)
> please patch both the 1.7 branch and the trunk.

Attachment 150366 [details] [diff] is against the trunk.
Comment on attachment 150366 [details] [diff] [review]
patch

I don't believe that I have reviewing rights for themes.
Attachment #150366 - Flags: review?(smontagu) → review?(neil.parkwaycc.co.uk)
you might want to add a comment there explaining why you use -moz-margin- and
not the standard one, so 3rd party theme creators won't be tempted to go back to
the old style.
I only change the places, where the left and the right margins are different.

In the first case, the right margin was 2px, the left - 0px.
In the second case, both the left and the right margins are equal (0px).

(If 4 margins are specified, they relate to the top, right, bottom and left 
respectively.)
However, besides menu.css, there are additional XUL-related style sheets that 
specify absolute left / right values. I believe these also should be fixed. - 
But let's see if the patch for menu is reviewed first.
[in reply to comment #20]

If the changes are approved, I think it's a good idea to put such a comment, at 
least somewhere at http://themes.mozdev.org.
[In reply to comment #21]

Sorry, you're right, it makes sense to use the -moz-* properties even though 
the left and right margins are identical.
mozilla 1.7 still uses the themes created for 1.5. i applied this patch to the
RTL-Modern and RTL-Classic themes i made for 1.5 (and used also for 1.6), and
discovered that it disrupts the display of the dropmarkers in the toolbar. in
Classic, they are too wide, and the image is aligned left, and if the mouse is
over the Back arrow and not directly over the dropmarker, the image completely
disappears.

in Modern, the Back dropmarker is compressed to an elipse, and the distance
between the buttons is too big.

i tried to see if menu.css styles the toolbar, but couldn't see any using DOM
Inspector. i just need a direction of how this patch affects the toolbar.

my RTL themes are basicly identical to the originals, eccept that i flipped some
of the graphics, and swaped the words "right" and "left" in many places in the
css. they can be obtained at
https://sourceforge.net/project/showfiles.php?group_id=42516&package_id=74309&release_id=193603
[In reply to comment #26]

Using the original mozilla themes, neither of the problems you mentioned seems 
to be reproducible. Please check with 1.7:

1. Do the standard themes misbehave with attachment 150366 [details] [diff] [review] applied?
2. Do your skins display okay without the patch?

And please beware that even a single place, where you swapped "right" 
and "left", can make the patch totally unusable.
Tsahi, I don't think menu.css affects the toolbar.
It seems that you just need to sync with the trunk.
For example, some problems of RTL-Classic can be fixed using the current
toolbarbutton.css; please see the diffs in the attachment.
This bug looks very similar to bugs #140759 and #185233
I suppose they are closely related, and probably the same problem cause the 3 bugs.
Tsahi, has there been any progress with the RTL* themes?
sorry lina, i was very busy recently and didn't have time for this. what is did
so far is this:

took the mozilla 1.5 classic and modern themes, which were also used in 1.6, and
in menu.css, where you replaced *-left with -moz-*-start, i replaced it with
*-right, so the themes are still compatible with 1.5. in addition, i applied
attachment 150969 [details] [diff] [review] to the classic theme, and in a similar manner modified the
modern theme. i then used these themes with mozilla 1.7 with the hebrew language
pack, and the problem was solved (WinXP).

i noticed that despite the fact that mozilla 1.7's theme version is 1.5, it has
a few more files than the themes in mozilla 1.6. i didn't have time to actually
modify the themes that come with 1.7.


(In reply to comment #29)
> This bug looks very similar to bugs #140759 and #185233
> I suppose they are closely related, and probably the same problem cause the 3
bugs.

althoug visually they may look the same, it's pretty clear to me that in the
source of it, the problem is with the CSS of the theme. i think mozilla should
be able to select the right CSS and graphics in the theme, according to the
direction of the language pack, as i wrote in bug 221824.
is it possible to apply the patch for RTL classic toolbarbutton.css to the
mozilla code base, and similarly patch the modern theme, in order to save me
some of the work in converting the themes?
Simon, can you move the review request to someone else?
a few days ago i produced new RTL versions of the default themes, this time
based on the themes that come with mozilla 1.7/win32. among other things, i
manually applied the patch in attachment 150366 [details] [diff] [review] to menu.css, and it worked as
advertised.

classic: http://prdownloads.sourceforge.net/hebmoz/rtl-classic-moz-1.7.xpi?download
modern: http://prdownloads.sourceforge.net/hebmoz/rtl-modern-moz-1.7.xpi?download
Neil, what about the review here?
probably the right solution for this bug is to create RTL-aware themes, like we have in firefox and thunderbird (bug 221824). i think the infrastructure is there (global/global.dtd: locale.dir in the language pack), so someone just needs to fix the themes to use it. see also comment 34 above.
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
Assignee: mozilla → nobody
Component: Layout: Text → Themes
Keywords: rtl
Product: Core → SeaMonkey
QA Contact: layout.fonts-and-text → themes
(In reply to Tsahi Asher from comment #36)
> probably the right solution for this bug is to create RTL-aware themes, like
> we have in firefox and thunderbird (bug 221824). i think the infrastructure
> is there (global/global.dtd: locale.dir in the language pack), so someone
> just needs to fix the themes to use it. see also comment 34 above.

so close this bug in favor of bug 221824? (or dup)
Although bug 221824 is in the SeaMonkey product, it appears to have been hijacked for toolkit specific work. Something more appropriate might be:
Bug 311383 - add RTL theme support to SeaMonkey [helpwanted].

We know what needs to be done (basically just copy what TB and FX have already done). But SeaMonkey is a completely volunteer driven community project and we badly need someone with theming (and graphics) skills to step forward and volunteer to implement this.

Bits and pieces of RTL awareness have been landing once in a while as part of other patches. However our UI-module owner would prefer a more systematic effort rather than piecemeal efforts here and there.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 311383
Comment on attachment 150366 [details] [diff] [review]
patch

Cancelling review request due to extreme bit rot. This bug is too old and has too much history.
Please open a new bug with a new patch for the menu item alignment case. Assuming that this still occurs in Modern. Our Default (Classic) theme now picks up the global styles from Toolkit so I assume that RTL menu items don't have any problems in our Default theme.
Attachment #150366 - Flags: review?(neil)
You need to log in before you can comment on or make changes to this bug.