477590 - [Mac] Search widgets look wrong in RTL mode

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Markus Stange [:mstange]]

Attachment: Screenshot of searchfield in RTL mode

See bug 450800 comment 41 and 43: In RTL mode, both the search icon and the cancel button are on the left.
We should either draw the search icon at the logical start or lock the cancel button at the right.

Comment 1

[Dão Gottwald [:dao]]

(In reply to comment #0)
> We should either draw the search icon at the logical start or lock the cancel
> button at the right.

The former would be the ideal fix. The latter would be workaround, and while it would make the search field look balanced, I'm not sure if a cancel/clear button at the logical start makes sense interaction-wise.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Similar things can also be seen for rtl locales on Windows. The clear icon isn't shown at all. Markus, shall I better file a new bug on that? I think so because we are using a native widget on OS X.

Comment 3

[Markus Stange [:mstange]]

(In reply to comment #2)
> Similar things can also be seen for rtl locales on Windows. The clear icon
> isn't shown at all. Markus, shall I better file a new bug on that?

Yes, please.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Filed as bug 478517

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Markus, would you have time to fix it for FF3.1?

Comment 6

[(no longer active)]

Attachment: Patch (v1)

(In reply to comment #0)
> We should either draw the search icon at the logical start or lock the cancel
> button at the right.

This patch implements the former approach.  I haven't tested it myself though.  Could someone with a Mac please test it?

Comment 7

[Dão Gottwald [:dao]]

That patch is wrong in several ways. You're adding the padding on the wrong side; the search icon is internally still drawn on the left. And you're flipping the whole textbox, including the clear icon and the text content.

Comment 8

[(no longer active)]

(In reply to comment #7)
> That patch is wrong in several ways. You're adding the padding on the wrong
> side;

How come?  The right padding should be -moz-padding-end and the left padding should be -moz-padding-start, right?

> the search icon is internally still drawn on the left.

Hmm, does scaleX(-1) affect the paddings as well?  I was assuming that it doesn't...

> And you're
> flipping the whole textbox, including the clear icon and the text content.

Oops.  That should be corrected by specifying scaleX(1) on the textbox-input-box hbox, right?

Comment 9

[Dão Gottwald [:dao]]

(In reply to comment #8)
> (In reply to comment #7)
> > That patch is wrong in several ways. You're adding the padding on the wrong
> > side;
> 
> How come?  The right padding should be -moz-padding-end and the left padding
> should be -moz-padding-start, right?
> 
> > the search icon is internally still drawn on the left.
> 
> Hmm, does scaleX(-1) affect the paddings as well?  I was assuming that it
> doesn't...

It takes the whole thing and draws it mirrored.

> > And you're
> > flipping the whole textbox, including the clear icon and the text content.
> 
> Oops.  That should be corrected by specifying scaleX(1) on the
> textbox-input-box hbox, right?

textbox-input-box is right, but scaleX(1) would be a no-op.

Comment 10

[Markus Stange [:mstange]]

Ehsan, may I take this bug? Sorry that I didn't get to it earlier.
I'm working on a patch that doesn't need additional CSS styles and instead mirrors the control in nsNativeThemeCocoa.mm.

Comment 11

[Markus Stange [:mstange]]

Attachment: wip fix, v1

I haven't tested it on 10.4, but this should be relatively complete.

Comment 12

[(no longer active)]

(In reply to comment #10)
> Ehsan, may I take this bug? Sorry that I didn't get to it earlier.
> I'm working on a patch that doesn't need additional CSS styles and instead
> mirrors the control in nsNativeThemeCocoa.mm.

Sure, what you're doing is much better. :-)

Comment 13

[Markus Stange [:mstange]]

Attachment: v2, better code

Comment 14

[Markus Stange [:mstange]]

Attachment: screenshot with patch

With the patch, all controls that we draw with NSCells will be mirrored for RTL frames, even dropdowns and checkboxes. Is that the right thing to do?

Comment 15

[(no longer active)]

(In reply to comment #14)
> Created an attachment (id=372227) [details]
> screenshot with patch
> 
> With the patch, all controls that we draw with NSCells will be mirrored for RTL
> frames, even dropdowns and checkboxes. Is that the right thing to do?

Probably not.  I'm not familiar with RTL conventions on Mac, but at least on Linux and Windows, some controls are mirrored in this way (like drop-downs for example), but things like checkboxes do not get mirrored.

I think you need to check what other RTL apps on Mac do...

Comment 16

[Markus Stange [:mstange]]

(In reply to comment #15)
> I think you need to check what other RTL apps on Mac do...

I have no idea how to do that. I've tried selecting different RTL languages in the System International preferences, but that didn't change anything, probably because those languages aren't supported by any app that I tested. And AFAICT there's no RTL language among the ones that Mac OS X supports natively:
http://support.apple.com/kb/HT3155

Yehuda, you've done Mac RTL stuff before, can you help us out?

Comment 17

[Yehuda]

(In reply to comment #16)

> AFAICT there's no RTL language among the ones that Mac OS X supports natively:
> http://support.apple.com/kb/HT3155
> 

This is correct. Apple does not supply out-of-the-box support for RTL languages. Support for Hebrew is developed and distributed separately by iDigital - the local Apple distributor. There might be similar packages for Arabic and Farsi.

I never installed this package because: 
A. It looks like a Beta - with many elements still drawn the wrong way. (e.g. the folders on the left in Mail and Finder as seen in this screen-shot: http://skitch.com/ziv_kitaro/i16i/1)
B. I'm too used to the LTR layout.

From looking at the screen-shots around the net, it looks like some of the controls are mirrored and some are not. Ehsan is right about the checkbox. It looks wrong when mirrored. 

See more screenshots here:
http://osx86heb.darkbb.com/portal-110.htm#news_110

> Yehuda, you've done Mac RTL stuff before, can you help us out?
Sure. Let me know if you need anything else.

Comment 18

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #17)

> This is correct. Apple does not supply out-of-the-box support for RTL
> languages. Support for Hebrew is developed and distributed separately by
> iDigital - the local Apple distributor. There might be similar packages for
> Arabic and Farsi.

Indeed, see http://www.emiratesmac.com/forums/blog/5090-arabic-leopard-out.html for Arabic.

IB3 on 10.5 supposedly supports a RTL pop-up button widget, but I've never been able to coax it into appearing, so I assume no native apps are using it (Mellel, at least, does not appear to, though I think it still supports 10.4, anyway).

IMO, the pop-up button and combobox controls probably make the most sense to flip, since right-aligned text flush up against the blue endcap just looks "wrong" to me (regular buttons you flip, of course, but since they're symmetrical, they're not noticeable).  But my rule has generally been "what does Mellel do?" ;)

Comment 19

[(no longer active)]

Based on the comments posted here, I think we need a new patch which doesn't flip checkboxes but continues to flip drop-downs.

Comment 20

[Markus Stange [:mstange]]

Attachment: v3, don't flip checkboxes

Comment 21

[Markus Stange [:mstange]]

Attachment: v3, don't flip checkboxes

Oops, forgot to qrefresh.

Comment 22

[Markus Stange [:mstange]]

Attachment: basic reftest

Comment 23

[Markus Stange [:mstange]]

Attachment: for checkin (patch + test)

Tryserver told me that the searchfield-mirrored-when-rtl.xul test fails on both Windows and Linux, so I've enabled it only for Mac.

Comment 24

[Dão Gottwald [:dao]]

You need to update your tree and merge in textbox.css.

Comment 25

[Markus Stange [:mstange]]

Attachment: updated, for checkin

Comment 26

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/223b234d0e1b

Comment 27

[Dão Gottwald [:dao]]

We should probably take this on branch, because of the rtl improvements and because it makes it easier for themes to use -moz-appearance: searchfield, which has been introduced in beta 3.

Comment 28

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 373107 [details] [diff] [review]
updated, for checkin

a191=beltzner

Comment 29

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fe15a4c29788

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; fa; rv:1.9.2a1pre) Gecko/20090419 Minefield/3.6a1pre ID:20090419030918

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; fa; rv:1.9.1b4pre) Gecko/20090419 Shiretoko/3.5b4pre ID:20090419031029

The search icon in the search bar is still a bit mis-aligned. I'll file a new bug on this issue.