240485 - Bookmarks Toolbar items go under the 'chevron' icon, when interface is RTL

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Reuven Gonen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; he-IL; rv:1.6) Gecko/20040206 Firefox/0.8

When the interface direction is RTL as in Hebrew or Arabic, the bookmarks on the
'Bookmarks Toolbar Folder' go under the 'chevron' icon(the icon looks like this
'>>' ,This icon is actually used when they are Bookmarks that don't fit on the
toolbar).

Reproducible: Always
Steps to Reproduce:
1. Make sure they are a few bookmarks on the Bookmarks Toolbar Folder. Also make
sure the toolbar is displayed(if not go to view->toolbars->bookmarks toolbar).

2.Add this code to the userChrome.css file To chanage the direction to RTL(it's
under the chrome folder inside the user profile.

/*make UI RTL */

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

menu { direction: rtl; }

outliner { direction: rtl; }

/*************************************
**  Fix address bar directionality  **
*************************************/

/*make address bar remain LTR */

#urlbar { direction: ltr !important; }

/* leave search from address bar in RTL */

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

/***** End address bar fix ******/

/* Align text in menulist pupop items to right  */

menulist > menupopup > menuitem {
  padding: 1px 2px 1px 30px !important;
}

/* Align autocomplete popups items in urlbar to left in firefox */

popup[type="autocomplete"],
.autocomplete-history-popup {
 direction: ltr !important;
}

3. Open the Firefox browser.

Actual Results:  
the Bookmarks Toolbar items are aligned to the left under the '>>' icon

Expected Results:  
the bookmarks should be aligned to the right. if they are bookmarks that don't
fit on the toolbar, they should go under the '>>' icon on the left

the bookmarks toolbar is defined as "PersonalToolbar" in xul and css.

Comment 1

[Reuven Gonen]

Attachment: Bookmarks toolbar

Comment 2

[Mike Connor [:mconnor]]

do we have existing localizations that are affected by this?  i.e. the available
Hebrew localization at
http://www.mozilla.org/products/firefox/releases/#download, is that affected by
this?  I haven't seen any bugs on that, and 0.8 has been out for a couple months
now.

Comment 3

[Tsahi Asher]

the current language pack for firefox aligns the bookmarks toolbar to the left,
while the rest if the UI is aligned to the right, in order to avoid this bug.
therefore, users don't see this. but we would like to be able to align the
bookmarks toolbar to the right as well.

Seamonkey had a similar problem, which was only partialy fixed, at bug 218528.

Comment 4

[Asaf Romano (gone)]

Attachment: Probably the fix...

Fixes the bug using WIDTHes instead of horizontal x values

Comment 5

[Tsahi Asher]

when this was partialy fixed in bug 218528, they left a problem in which draging
links to the toolbar droped them one place left of what you expected (left of
where the black vertical line is). did you check that you can drop new links to
the toolbar or drag items around with no problem?

Comment 6

[Asaf Romano (gone)]

(In reply to comment #5)
> when this was partialy fixed in bug 218528, they left a problem in which draging
> links to the toolbar droped them one place left of what you expected (left of
> where the black vertical line is). did you check that you can drop new links to
> the toolbar or drag items around with no problem?

I checked it with the patch and I don't see any problem with dropping, adding
and reordering the bookmarks toolbar.

Comment 7

[Asaf Romano (gone)]

(In reply to comment #6)
> (In reply to comment #5)
> > when this was partialy fixed in bug 218528, they left a problem in which draging
> > links to the toolbar droped them one place left of what you expected (left of
> > where the black vertical line is). did you check that you can drop new links to
> > the toolbar or drag items around with no problem?
> 
> I checked it with the patch and I don't see any problem with dropping, adding
> and reordering the bookmarks toolbar.

ok soory, this specific strange behavior still happens. I'll try to find what's
the reason for this. but i'm not promising anything :-)

Comment 8

[Asaf Romano (gone)]

Comment on attachment 146499 [details] [diff] [review]
Probably the fix...

Remove review request until i work around the dragging problem

Comment 9

[Mike Connor [:mconnor]]

not a 0.9 blocker.  A patch that doesn't cause regressions would be accepted,
but this isn't critical for a feature-driven release.  1.0beta is where the
cleanup stuff belongs.

Comment 10

[Asaf Romano (gone)]

(In reply to comment #9)
> not a 0.9 blocker.  A patch that doesn't cause regressions would be accepted,
> but this isn't critical for a feature-driven release.  1.0beta is where the
> cleanup stuff belongs.

If you were using the HebrewL10N version of Firefox, you wodn't denay it so
quickly, it's realy annoying...

and.. just a litle correctioon: there is no regression in the patch. The drag &
drop problem applies only for RTL bookmarks-bar.

Comment 11

[Mike Connor [:mconnor]]

if I was using the Hebrew l10n I'd probably have fixed this myself a long time
ago. :)

A partial fix that doesn't hose LTR at all is iffy, since we're basically saying
that RTL has to put up with a buggy interface to have the bookmarks orient RTL.

re-request review, we'll see what the peers say, if its really a net
improvement, okay, but having unexpected behaviour is a bad thing.

its still not a blocker, since its not like its something that's provoked a
massive backlash with users of that l10n, and we're still working towards 1.0. 
If you can figure out a workaround (or even a cheap wallpaper hack) to make this
work properly, the chances improve :)

Comment 12

[Asaf Romano (gone)]

Attachment: Fixes the bug (and the Drag & Drop issue)

OK, new patch is fixing the bug and drag & drop issue.

seems to be final...

Comment 13

[Asaf Romano (gone)]

Attachment: Final Patch

Much better solution for working around this annoying bug (no change for the
code in LTR cases and no more problems with non-stanard themes or strange
userChrome...).

Comment 14

[Asaf Romano (gone)]

Asking again for blocking (now that there is a patch)

Comment 15

[Mike Connor [:mconnor]]

still not a blocker, but I'll review this later if you request the review from
me (instead of from no one)

Comment 16

[Simon Montagu :smontagu]

Comment on attachment 147985 [details] [diff] [review]
Final Patch

the patch looks OK from the Bidi point of view.

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 147985 [details] [diff] [review]
Final Patch

removing DROP_ON means that  you can't drop onto a folder now

Comment 18

[Mike Connor [:mconnor]]

Attachment: patch

this is essentially Asaf's patch, cleaned up in a few places, with DROP_ON
properly restored

Comment 19

[Mike Connor [:mconnor]]

Comment on attachment 148793 [details] [diff] [review]
patch

there's a couple minor nites wrt spacing in here, I'll fix that on checkin

Comment 20

[Asaf Romano (gone)]

(In reply to comment #17)
> (From update of attachment 147985 [details] [diff] [review])
> removing DROP_ON means that  you can't drop onto a folder now
> 

Oh, i missed it up 10x for correction :-)

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 148793 [details] [diff] [review]
patch

checked this in for 0.9, will do the spacing cleanup after.

Comment 22

[Mike Connor [:mconnor]]

fixed on trunk, finally.

Comment 23

[(no longer active)]

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