412017 - [RTL][GTK] Native themed location bar (drop down button) looks bad on RTL layout

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Teune van Steeg]

Attachment: fix RTL layout of location bar

When bug 405210 gets fixed, the location bar drop down button will be themed natively on GTK. The text field has an open end towards the drop down button, so the drop down button looks attached to the text field. However on RTL layouts the drop down button will be positioned to the left of the location bar, and the text field itself will have an open end towards the right (into nothing) because its styled direction is LTR.

This patch fixes this by moving the styled LTR direction (which is needed for a correct text layout in the location bar) one child node further.
This causes the buttons and favicons in the location bar to switch place, I don't know if that is correct layout, but what gave me the idea that this might be the best solution is that the RTL Go button gets used, which at the moment is not the case.

There are however other solutions of which I will attach a screenshot.

Comment 1

[Teune van Steeg]

Attachment: fix RTL layout of location bar

Oops, that was on old version of the real patch.

Comment 2

[Teune van Steeg]

Attachment: screenshot of the currrent situation and possible solutions

In this screen shot the top location bar is how it will look like on RTL layout when the patch for bug 405210 is commited.

Below that there are three options to fix the layout of the location bar in that case:
1) style the whole location bar LTR by default
2) style the drop down button and the text field border RTL, but let the content be layed out LTR.
3) my solution: only style the input field LTR and the rest RTL.

The current situation, without the patch for bug 405210, is that of option 2.

Comment 3

[Asaf Romano (gone)]

Comment on attachment 296633 [details] [diff] [review]
fix RTL layout of location bar

r=mano, Thanks!

Comment 4

[Asaf Romano (gone)]

But please fix winstripe too...

Comment 5

[Teune van Steeg]

Attachment: fix RTL layout of location bar (including winstripe).

> But please fix winstripe too...
Not sure if I should request for review again, but here you go!

Comment 6

[Reed Loden [:reed]]

Comment on attachment 296643 [details] [diff] [review]
fix RTL layout of location bar (including winstripe).

Fix RTL issue with location bar on gtk (but also an issue on winstripe).

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 296643 [details] [diff] [review]
fix RTL layout of location bar (including winstripe).

I think this is wrong. The size and number of urlbar icons varies. So this will shift the address.

Comment 8

[Teune van Steeg]

Comment on attachment 296631 [details] [diff] [review]
fix RTL layout of location bar

(In reply to comment #7)
> (From update of attachment 296643 [details] [diff] [review])
> I think this is wrong. The size and number of urlbar icons varies. So this will
> shift the address.
> 
Then the version I had in mind first might be the best solution (the second option mentioned in comment #2, the third in the screenshot)

Comment 9

[Dão Gottwald [:dao]]

(In reply to comment #8)
> Then the version I had in mind first might be the best solution (the second
> option mentioned in comment #2, the third in the screenshot)

In the third picture, there appears to be a margin or padding between the dropmarker and the site button (the box with the favicon). Could you please take a look at where this is coming from?

Also, the arrow within the dropmarker isn't centered. Is this a bug in the widget code?

Comment 10

[Teune van Steeg]

(In reply to comment #9)
> (In reply to comment #8)
> > Then the version I had in mind first might be the best solution (the second
> > option mentioned in comment #2, the third in the screenshot)
> 
> In the third picture, there appears to be a margin or padding between the
> dropmarker and the site button (the box with the favicon). Could you please
> take a look at where this is coming from?
> 
This is because the open border of the entry widget is created by the GTK theme. It replaces the normal border with what the theme assumes must be the text field background color (white). There is AFAIK nothing we can do about that. 

> Also, the arrow within the dropmarker isn't centered. Is this a bug in the
> widget code?
> 
That could be, I will look into that.

Comment 11

[Dão Gottwald [:dao]]

(In reply to comment #10)
> This is because the open border of the entry widget is created by the GTK
> theme. It replaces the normal border with what the theme assumes must be the
> text field background color (white). There is AFAIK nothing we can do about
> that.

Does it assume the wrong background even in LTR mode if you're on a secure site (i.e. yellow urlbar)?

Comment 12

[Teune van Steeg]

(In reply to comment #11)
> Does it assume the wrong background even in LTR mode if you're on a secure site
> (i.e. yellow urlbar)?
> 
Yes, it does :(

Comment 13

[Dão Gottwald [:dao]]

So you really see no way to work around this? Drawbacks like this spoil the native theming, and I think it's bad enough to block bug 405210. I don't like the current dropmaker for various reasons, but it could be improved without native theming.

Comment 14

[Teune van Steeg]

This can be fixed with the following style rule (and current patch):

>#urlbar > .autocomplete-textbox-container {
>  -moz-margin-end: -2px;
>}

Shall I include this in the current patch, or should that be fixed in bug 405210?

Also I found the arrow centering is a bug in the widget code. So I guess a new patch is required there anyway.

Comment 15

[Dão Gottwald [:dao]]

(In reply to comment #14)
> This can be fixed with the following style rule (and current patch):
> 
> >#urlbar > .autocomplete-textbox-container {
> >  -moz-margin-end: -2px;
> >}
> 
> Shall I include this in the current patch, or should that be fixed in bug
> 405210?

This belongs to bug 405210, and please ask a toolkit peer for review.

Comment 16

[Asaf Romano (gone)]

Comment on attachment 296631 [details] [diff] [review]
fix RTL layout of location bar

and winstripe?

Comment 17

[Dão Gottwald [:dao]]

winstripe doesn't have the problem described in comment 0 (due to bug 405210).

Comment 18

[Reed Loden [:reed]]

Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.154; previous revision: 1.153
done

Comment 19

[(no longer active)]

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