Closed Bug 785952 Opened 8 years ago Closed 8 years ago

arrow panel issues with side arrows and RTL

Categories

(Toolkit :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla18
Tracking Status
firefox17 + verified

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Details

(Whiteboard: [Fx17])

Attachments

(5 files, 1 obsolete file)

Attached image 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
Attached image firstopen-side.png
Attached image secondopen-side.png
this shows the correct placement of the arrow happens for any opening after the first time.
Attached image rtl-open.png
showing misplacement of arrow image when LTR
Whiteboard: [Fx17]
Attached patch popup-rtl.patch (obsolete) — Splinter Review
fixes the position of the arrow for rtl
Component: SocialAPI → General
Product: Firefox → Toolkit
Summary: flyout panel arrow issues → arrow panel issues with side arrows and RTL
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.
btw, the "firstopen-top.png" image only lined up with the search icon by coincidence, I am now having an issue reproducing that issue.
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.
Assignee: nobody → mixedpuppy
Attachment #655764 - Attachment is obsolete: true
Attachment #656057 - Flags: review?(gavin.sharp)
Attachment #656057 - Flags: review?(gavin.sharp) → review?(enndeakin)
Attachment #656057 - Flags: review?(enndeakin) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8c8d85acebce
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
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.
Attachment #656057 - Flags: approval-mozilla-aurora+
Keywords: verifyme
QA Contact: anthony.s.hughes
What are some RTL locales I can use to test this bug?
The three main rtl locales are arabic, hebrew and persian.
Verified fixed with the latest Firefox 17.0a2 and 18.0a1 arabic builds.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.