631152 - Search autocomplete popup opens in the wrong position in RTL mode

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[(no longer active)]

Attachment: Screenshot

See the screenshot.

Comment 1

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

Ehsan, does it happen on all platforms? Would be good to know between which beta releases it has been regressed.

Comment 2

[Anas Husseini [:linostar]]

Attachment: Screenshot of the search autocomplete popup on linux x64

Comment 3

[Anas Husseini [:linostar]]

I've tested on both linux and win32 platoforms. The popup appears in its right place there. See the second screenshot.

Comment 4

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

So this is an OS X only issue? Ehsan, are you using multiple screens?

Comment 5

[(no longer active)]

(In reply to comment #4)
> So this is an OS X only issue? Ehsan, are you using multiple screens?

I haven't tested on other platforms, and no, I wasn't been using multiple screen.

Comment 6

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

Also happens in 3.6.13 builds of ar on OS X. I'm sure this is a dupe of another bug.

Comment 7

[(no longer active)]

I could reproduce this on Hebrew trunk builds on all platforms.

Comment 8

[(no longer active)]

Attachment: Patch (v1)

We shouldn't really think that we can position the popup better than the xul popup manager code!

Comment 9

[(no longer active)]

Henrik, please dupe any other bugs on this you can find against this one, as it has a patch.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 509525 [details] [diff] [review]
Patch (v1)

This method really needs to be synced with autocomplete.xml's _openAutocompletePopup (and I don't think you want to be removing the width setting). We may be able to do that by just removing this override entirely, but we need to make sure not to regress bug 398020 comment 24 (aligning the popup with the text field itself, rather than the engine selector dropdown).

Comment 11

[(no longer active)]

Attachment: With patch, LTR

Comment 12

[(no longer active)]

Attachment: With patch, RTL

Comment 13

[(no longer active)]

Attachment: Patch (v2)

Comment 14

[(no longer active)]

Actually it's not a regression...

Comment 16

[Anas Husseini [:linostar]]

(In reply to comment #14)
> Actually it's not a regression...

Are you sure about that? In the second attachment, taken from a linux64 build of fx4 beta that was built a couple of weeks ago, the popup appears in its proper place. However, I have just made a new linux64 build where I've found that the popup is shifted from its expected place. It seems that the cause was probably landed in mozilla-central in that short period.

Comment 17

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

Do you have a closer time range for Linux?

Comment 18

[(no longer active)]

(In reply to comment #16)
> (In reply to comment #14)
> > Actually it's not a regression...
> 
> Are you sure about that? In the second attachment, taken from a linux64 build
> of fx4 beta that was built a couple of weeks ago, the popup appears in its
> proper place. However, I have just made a new linux64 build where I've found
> that the popup is shifted from its expected place. It seems that the cause was
> probably landed in mozilla-central in that short period.

Yes, I'm sure (judging by reading the faulty code, and how long it goes back, and actually testing on 3.6!).

Please note that according to the exact place of the Firefox window on the screen, there is a slight possibility that you would see the autocomplete popup in the right position, but that's an exception, and moving the window on the screen reveals the problem.

Comment 19

[(no longer active)]

Comment on attachment 509634 [details] [diff] [review]
Patch (v2)

Gavin: I think this is a nice fix to take for RTL locales.  It's not a blocker by any means, but if you had a few spare cycles, I'd really appreciate if you can take a look at this.

Comment 20

[(no longer active)]

Gavin: ping?

Comment 21

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 509634 [details] [diff] [review]
Patch (v2)

Can you add a comment to the top of this method along the lines of:

"This method overrides the autocomplete binding's openPopup (essentially duplicating the logic from the autocomplete popup binding's openAutocompletePopup method), modifying it so that the popup is aligned with the inner textbox, but sized to not extend beyond the search bar border."

Comment 22

[(no longer active)]

Attachment: For check-in

Comment 23

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/be9814a5a456

Comment 24

[Vlad [QA]]

Verified on Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110403 Firefox/4.2a1pre

Search autocomplete popup opens in the wright position.