Tomer Cohen :tomer Reporter
	Description • 7 years ago

The RTL buttons of the compact theme seems to miss a left border on RTL builds of Firefox. I am attaching a screenshot that demonstrate the current state and the purposed fix. 

I was able to produce the expected result by commenting out the #urlbar { border-inline-start: none !important; } from compacttheme.css, but I am unsure if this won't break something else in the process.

	Tomer Cohen :tomer Reporter
	Comment 1 • 7 years ago

An addition:

removing the #back-button > .toolbarbutton-icon, #forward-button > .toolbarbutton-icon {
border: 1px solid var(--chrome-nav-bar-controls-border-color); } from compacttheme.css seems to resolve the 2px mid border between the buttons.

	:Gijs (he/him)
	Comment 3 • 7 years ago

This turns out to be really annoying to test, because using force rtl with the English build doesn't produce the same result as in either of your screenshots.

In any case, I don't think the fix you're looking at is right. Something like this seems more likely to be correct:

https://pastebin.mozilla.org/8970169

Note that I'm basically replacing all the padding/margin/border-(left|right) with -inline-(start|end) instead, except for the transition which doesn't seem to work (so I'm specialcasing it based on the direction.

Can you check if this patch works for you? You should be able to download it and then use 'hg import' (though you might need to fix the newlines with dos2unix before it'll work).

	Tomer Cohen :tomer Reporter
	Comment 4 • 7 years ago

I am facing few difficulties, as I am able to fix it on DevTool, but then when I'm build a try build I see no difference.

	Tomer Cohen :tomer Reporter
	Comment 6 • 7 years ago

(In reply to :Gijs from comment #3)
> This turns out to be really annoying to test, because using force rtl with
> the English build doesn't produce the same result as in either of your
> screenshots.

I'm getting the same [bad] results with your patch applied as well.
https://archive.mozilla.org/pub/firefox/try-builds/tomer.mozilla@tomercohen.com-78d852af09b0852d65d95d7599af586367b4a15c/

	:Gijs (he/him)
	Comment 7 • 7 years ago

So, I'm confused. Do you see this problem on a clean profile? Does it work correctly on the default theme on a clean profile?

The real bug here is that the forward button in your screenshot is getting a border on the right-hand side instead of the left-hand side. There's already a (lighter, because the back button is disabled) border on the right-hand side of the forward button that is generated by the back button. We shouldn't need to mess with anything except making sure that the current right-hand-side border on the forward button appears on the left-hand-side instead.

	:Gijs (he/him)
	Comment 8 • 7 years ago

Maybe try changing this:

http://searchfox.org/mozilla-central/source/browser/themes/linux/compacttheme.css#57

which has: border-inline-start: none !important;

to border-left: none !important;

and see if that helps?