785952 - arrow panel issues with side arrows and RTL

Status: VERIFIED FIXED

(Toolkit :: General, defect)

Description

[Shane Caraveo (:mixedpuppy)]

Attachment: firstopen-top.png

This is actually a regular panel bug, but it affects social directly.  I haven't seen any other panels in firefox that use side arrows.  I'll attach images and a patch that I believe fixes the issue.

1. first open misplaces or doesn't show the arrow
2. using the force rtl addon, the arrow is on the wrong side

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: firstopen-side.png

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: secondopen-side.png

this shows the correct placement of the arrow happens for any opening after the first time.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: rtl-open.png

showing misplacement of arrow image when LTR

Comment 4

[Shane Caraveo (:mixedpuppy)]

Attachment: popup-rtl.patch

fixes the position of the arrow for rtl

Comment 5

[Shane Caraveo (:mixedpuppy)]

we open the side panel here:

https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-social.js#290

and the status panels are opened here:

https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-social.js#554

I have had this issue in entirely different code bases as well.  

For the initial open of a side panel, where the arrow is not shown at all, that is fixed if you set the side attribute on the arrowpanel element prior to opening the panel.

Comment 6

[Shane Caraveo (:mixedpuppy)]

btw, the "firstopen-top.png" image only lined up with the search icon by coincidence, I am now having an issue reproducing that issue.

Comment 7

[Neil Deakin]

The patch here looks ok, although I would change it so as not to set the 'dir' attribute on 'container' twice.

> For the initial open of a side panel, where the arrow is not shown at all,
> that is fixed if you set the side attribute on the arrowpanel element prior
> to opening the panel.

Yes, that's a known issue as the side attribute defaults to 'top' and implies a vertical arrangement. Setting the side attribute is suitable here, although you should just set it in the markup.

Comment 8

[Shane Caraveo (:mixedpuppy)]

Attachment: arrow panel rtl patch

Comment 9

[Shane Caraveo (:mixedpuppy)]

try: https://tbpl.mozilla.org/?tree=Try&rev=7e45fdec7cf3

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c8d85acebce

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8c8d85acebce

Comment 12

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

Comment on attachment 656057 [details] [diff] [review]
arrow panel rtl patch

[Triage Comment]
This is a simple RTL fix that impacts bug 779923's functionality (new in 17), so let's get this on Aurora as well.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1a15b9f66b41

Comment 14

[u279076]

What are some RTL locales I can use to test this bug?

Comment 15

[Neil Deakin]

The three main rtl locales are arabic, hebrew and persian.

Comment 16

[u279076]

Verified fixed with the latest Firefox 17.0a2 and 18.0a1 arabic builds.