Closed
Bug 1204366
Opened 10 years ago
Closed 8 years ago
[RTL] Searchbar's dropdown in wrong positions
Categories
(Firefox :: Search, defect, P2)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: mvocom, Assigned: florian)
References
Details
(Keywords: rtl, Whiteboard: [fxsearch][RTL])
Attachments
(4 files, 2 obsolete files)
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150826023504
Steps to reproduce:
In RTL locals (Hebrew etc.), the Searchbar's dropdown is not aligned properly.
http://s3.postimg.org/xzplttnn7/FF_En.png
http://s4.postimg.org/flx4tmjd9/FF_He.png
Comment 1•10 years ago
|
||
Works for me. Can you reproduce this when using the default theme?
Flags: needinfo?(mvocom)
Change text direction (Ctrl+Shift+X).
http://s14.postimg.org/qjpyb3qht/FF_He_Default_Theme.png
Searching mostly in English, I've added "#search-container .textbox-input-box { direction: ltr !important; }" to userChrome.css.
The problem exists in the Old Search UI (much better IMHO) as well, but not to the same degree.
I think the code in search.xml needs to be modified.
const isRTL = getComputedStyle(this, "").direction == "rtl";
...
Thank you.
Flags: needinfo?(mvocom)
Note also the difference between opening the dropdown with a value and without one.
http://s22.postimg.org/bfd35kkkx/Value.png
http://s28.postimg.org/6qyz9bxq5/No_Value.png
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Yaron from comment #2)
> Change text direction (Ctrl+Shift+X).
> http://s14.postimg.org/qjpyb3qht/FF_He_Default_Theme.png
>
> Searching mostly in English, I've added "#search-container
> .textbox-input-box { direction: ltr !important; }" to userChrome.css.
Is the problem only happening when you change something in userChrome.css?
The problem occurs when you change text direction to LTR (regardless of userChrome.css).
I've added that line to save myself changing the direction manually.
I assume your UI is LTR.
Could you please change text direction in the Search bar and check how the dropdown is displayed?
(I forgot to do that when I changed my UI).
Thank you.
Comment 6•10 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:43.0) Gecko/20100101 Firefox/43.0
20150914030205
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0
20150826023504
(In reply to Yaron from comment #2)
> Change text direction (Ctrl+Shift+X).
In that case, I can reproduce the issue.
(In reply to Yaron from comment #5)
> I assume your UI is LTR.
I tested the Hebrew versions of Firefox 40.0.3 and the latest Nightly, in a brand new profile.
Thank you Gingerbread Man.
The problem also occurs in LTR UI if you change direction in the Search bar to RTL.
Updated•10 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P4
Whiteboard: [fxsearch]
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8660550 [details]
works for me.png
This screenshot shows another problem. There should be vertical bars on both sides of the wikipedia icon, and there should be one only on one side of the Yahoo icon.
Hi Florian,
Nice observation. :)
Can you take this bug (the main issue)?
If not, could you please think of someone else who would?
I'd like to emphasize that - as I've mentioned earlier - this bug is relevant to LTR UI as well (i.e. if you change direction in the Search bar to RTL).
I think this is a major issue.
Would it be possible to keep the old Search UI as long as it's not fixed?
***
search.xml:
popup.openPopup(this.inputField, "after_start", 0, yOffset, false, false);
If you replace "after_start" with "after_end", "direction: ltr" - displayed properly and vice versa (checked in RTL UI; I assume it would be the other way around in LTR UI).
Thank you.
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Yaron from comment #9)
> Can you take this bug (the main issue)?
I tried for a few minutes yesterday, but couldn't find an obvious fix.
> If not, could you please think of someone else who would?
I don't know right now. I'll try to think about it.
> Would it be possible to keep the old Search UI as long as it's not fixed?
The old search UI is already removed, so no.
> ***
>
> search.xml:
> popup.openPopup(this.inputField, "after_start", 0, yOffset, false, false);
>
> If you replace "after_start" with "after_end", "direction: ltr" - displayed
> properly and vice versa (checked in RTL UI; I assume it would be the other
> way around in LTR UI).
Is this a proposed fix, or a way to reproduce? I'm confused.
Priority: P4 → P2
Reporter | ||
Comment 11•10 years ago
|
||
Thank you Florian. I appreciate it.
If you replace "after_start" with "after_end"...
In my RTL UI, if the current direction is in the Search bar is LTR - the popup is displayed properly; - if the current direction is RTL, the popup is NOT displayed properly (the behavior is reversed).
So, It might be a direction to a fix.
- Theoretically, if you check the *current* direction before opening the popup and then accordingly use either "after_start" or "after_end", it might fix the issue.
Another possible direction is:
popup.openPopup(null, "after_start", ???, outerRect.bottom, false, false);
x position - I don't know.
"after_start" is ignored.
Regards.
Reporter | ||
Comment 12•10 years ago
|
||
Another issue:
The "Search with current engine" segment is a command, and should have a different background for hover and hover:active.
http://s2.postimg.org/imqbhdkux/Search_With_Current.png
And shouldn't "Search settings" have a different background for hover:active?
***
Should I file a separate bug?
Summary: Searchbar's dropdown in wrong positions (in RTL) → Searchbar's dropdown in wrong positions
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Yaron from comment #12)
> Another issue:
>
> The "Search with current engine" segment is a command, and should have a
> different background for hover and hover:active.
>
> http://s2.postimg.org/imqbhdkux/Search_With_Current.png
>
> And shouldn't "Search settings" have a different background for hover:active?
>
> ***
> Should I file a separate bug?
These things are both unrelated to this bug, so yes, if you think something needs to be fixed/improved, please file a separate bug for each issue. Thanks!
Summary: Searchbar's dropdown in wrong positions → [RTL] Searchbar's dropdown in wrong positions
Reporter | ||
Comment 14•10 years ago
|
||
> Another possible direction is:
> popup.openPopup(null, "after_start", ???, outerRect.bottom, false, false);
> x position - I don't know.
popup.openPopup(null, -1, -outerRect.left, outerRect.bottom, false, false);
Solves the problem in RTL locals.
Please don't quit working on a better solution. :)
Updated•10 years ago
|
Rank: 22
Whiteboard: [fxsearch] → [fxsearch][RTL]
Assignee | ||
Comment 15•9 years ago
|
||
We are forcing the panel to have a width larger than the textfield's width using JS code. The alignment between the panel and the searchbox is obtained using a negative margin in searchbar.css. When the text field is in the normal direction (ie. same direction as the rest of the UI), margin-inline-start is used. When the textfield's direction has been reversed (eg. using Command+shift+X), we need to set margin-inline-end instead.
I assume this value will need to be different for each of the platforms, and I've only tested on Mac for now, so this WIP patch only changes the Mac CSS.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → florian
Status: NEW → ASSIGNED
Assignee | ||
Comment 16•9 years ago
|
||
Now with the Linux and Windows changes included.
Attachment #8758212 -
Attachment is obsolete: true
Attachment #8758365 -
Flags: review?(dao+bmo)
Comment 17•9 years ago
|
||
Comment on attachment 8758365 [details] [diff] [review]
Patch
(In reply to Florian Quèze [:florian] [:flo] from comment #15)
> We are forcing the panel to have a width larger than the textfield's width
> using JS code. The alignment between the panel and the searchbox is obtained
> using a negative margin in searchbar.css. When the text field is in the
> normal direction (ie. same direction as the rest of the UI),
> margin-inline-start is used. When the textfield's direction has been
> reversed (eg. using Command+shift+X), we need to set margin-inline-end
> instead.
Can you please add something along those lines as a documenting comment to the relevant CSS rule?
Also please double-check the numbers. I only tested on Linux where your margin-line-end appears to be off by one pixel...?
Attachment #8758365 -
Flags: review?(dao+bmo)
Assignee | ||
Comment 18•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #17)
> Also please double-check the numbers. I only tested on Linux where your
> margin-line-end appears to be off by one pixel...?
The whole panel is already off by one pixel on Linux for me. I purposefully kept it this way for the margin-line-end, so that the panel doesn't move at all when pressing Ctrl+shift+X. If you want to fix this off-by-one pixel issue, we'll need to change the margin-inline-start value too. And I think we set it this way to preserve the alignment between the glass icon and the search engine's icon.
Comment 19•9 years ago
|
||
(In reply to Florian Quèze [:florian] [:flo] from comment #18)
> (In reply to Dão Gottwald [:dao] from comment #17)
>
> > Also please double-check the numbers. I only tested on Linux where your
> > margin-line-end appears to be off by one pixel...?
>
> The whole panel is already off by one pixel on Linux for me. I purposefully
> kept it this way for the margin-line-end, so that the panel doesn't move at
> all when pressing Ctrl+shift+X. If you want to fix this off-by-one pixel
> issue, we'll need to change the margin-inline-start value too.
Please feel free to do that.
> And I think
> we set it this way to preserve the alignment between the glass icon and the
> search engine's icon.
This doesn't sound like the right trade-off to me. Why would we do this only on Linux anyway?
Assignee | ||
Comment 20•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #17)
> Also please double-check the numbers. I only tested on Linux where your
> margin-line-end appears to be off by one pixel...?
I only changed the margin-inline-start by 1px on Linux. On Windows the alignment looks correct on my machine. On Mac it's slightly off, but by less than 1px (there must be a rounding issue somewhere), and both the searchbar and the panel have rounded corners and shadows/large focus highlights, so it's not really visible.
Attachment #8758365 -
Attachment is obsolete: true
Attachment #8759152 -
Flags: review?(dao+bmo)
Updated•9 years ago
|
Attachment #8759152 -
Flags: review?(dao+bmo) → review+
Assignee | ||
Comment 21•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/4915ce29449ef3595957114ecdff8d036b72f238
Bug 1204366 - [RTL] Searchbar's dropdown in wrong positions, r=dao.
Comment 22•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Comment 23•9 years ago
|
||
I have reproduced this bug on firefox nightly according to()
The fixing bug has verified on Latest Nightly -- Build ID:(20160805004022),User Agent: Mozilla/5.0 (Windows NT 10.0; rv:50.0) Gecko/20100101 Firefox/50.0
Tested OS--Windows10 32bit
QA Whiteboard: [bugday-20160803]
Updated•9 years ago
|
Flags: qe-verify+
Comment 24•9 years ago
|
||
I managed to reproduce this issue on Firefox 39.0 (HE) build, under Windows 10 x64.
The issue was partially fixed, but the position of the Searchbar's dropdown it's not kept if changing the text direction in the Search-bar (Ctrl+Shift+x). The Searchbar's dropdown is moving 1px to Right/Left, according to the text direction.
The issue is reproducible on Firefox 50.0b6 (he)(fa)(ar)(en-US) builds, Firefox 52.0a1 (2016-10-10) and on Windows 10, x64, Mac OS X 10.11.6 and on Ubuntu 16.04 x64
Note that the issue is reproducible on Firefox 51.0a2 (2016-10-10) only on Ubuntu OS.
Here is a screencast of the issue: https://goo.gl/wwYavj
Since the position of the Searchbar's dropdown is not kept, I am reopening this issue.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 25•8 years ago
|
||
I can still reprocude this issue under latest Nightly (53.0a1 21.1.17).
Version: 40 Branch → unspecified
Assignee | ||
Comment 26•8 years ago
|
||
(In reply to ItielMaN from comment #25)
> I can still reprocude this issue under latest Nightly (53.0a1 21.1.17).
Which issue can you still reproduce? Is it the 1px move described in comment 24 (if so please open a new bug for it), or the original issue?
Flags: needinfo?(itiel_yn8)
Comment 27•8 years ago
|
||
(In reply to Florian Quèze [:florian] [:flo] from comment #26)
> Which issue can you still reproduce? Is it the 1px move described in comment
> 24 (if so please open a new bug for it), or the original issue?
Well actually I can reproduce both.
See attached GIF, demonstrating the 1px difference and the issue shown in attachment 8661049 [details].
Flags: needinfo?(itiel_yn8)
Comment 28•8 years ago
|
||
Assignee | ||
Comment 29•8 years ago
|
||
On your gif it looks like you can reproduce when clicking the glass icon, but not when typing and getting suggestions. Is that right? (If so, please file a new bug; we are not going to fix additional edge cases in this bug that has been resolved months ago.)
Comment 30•8 years ago
|
||
(In reply to Florian Quèze [:florian] [:flo] from comment #29)
> On your gif it looks like you can reproduce when clicking the glass icon,
> but not when typing and getting suggestions. Is that right? (If so, please
> file a new bug; we are not going to fix additional edge cases in this bug
> that has been resolved months ago.)
Yep, that's right, thanks for pointing that out.
Will open a another bug for it.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•