arrow panel issues with side arrows and RTL

VERIFIED FIXED in Firefox 17

Status

()

VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

unspecified
mozilla18
x86
macOS
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox17+ verified)

Details

(Whiteboard: [Fx17])

Attachments

(5 attachments, 1 obsolete attachment)

(Assignee)

Description

7 years ago
Created attachment 655646 [details]
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
(Assignee)

Comment 1

7 years ago
Created attachment 655648 [details]
firstopen-side.png
(Assignee)

Comment 2

7 years ago
Created attachment 655649 [details]
secondopen-side.png

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

Comment 3

7 years ago
Created attachment 655650 [details]
rtl-open.png

showing misplacement of arrow image when LTR
(Assignee)

Updated

7 years ago
Whiteboard: [Fx17]
(Assignee)

Comment 4

7 years ago
Created attachment 655764 [details] [diff] [review]
popup-rtl.patch

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
(Assignee)

Comment 5

7 years ago
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.
(Assignee)

Comment 6

7 years ago
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

7 years ago
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)

Comment 8

7 years ago
Created attachment 656057 [details] [diff] [review]
arrow panel rtl patch
Assignee: nobody → mixedpuppy
Attachment #655764 - Attachment is obsolete: true
Attachment #656057 - Flags: review?(gavin.sharp)
Attachment #656057 - Flags: review?(gavin.sharp) → review?(enndeakin)

Updated

7 years ago
Attachment #656057 - Flags: review?(enndeakin) → review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c8d85acebce
Flags: in-testsuite-
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8c8d85acebce
Status: NEW → RESOLVED
Last Resolved: 7 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+
tracking-firefox17: --- → +
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-aurora/rev/1a15b9f66b41
status-firefox17: --- → fixed
Keywords: checkin-needed
Keywords: verifyme
QA Contact: anthony.s.hughes
What are some RTL locales I can use to test this bug?

Comment 15

7 years ago
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
status-firefox17: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.