1027057 - Panel animations have wrong anchor in RTL mode

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

When using RTL mode, the new arrowpanel animations have their origin on the wrong side.

Comment 1

[Neil Deakin]

This is fairly simple. We just need to flip the transform-origins in rtl mode.

Comment 2

[Neil Deakin]

Attachment: Add rtl mode transform-origins

Comment 3

[Marco Mucci [:MarcoM]]

Added to Iteration 33.1

Neil, would you recommend this bug as [qa+] or [qa-] for verification?

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 8442946 [details] [diff] [review]
Add rtl mode transform-origins

>--- a/toolkit/content/xul.css
>+++ b/toolkit/content/xul.css

>+panel[arrowposition="start_before"]:-moz-locale-dir(ltr) {
>   transform-origin: right 20px;
> }

>+panel[arrowposition="end_before"]:-moz-locale-dir(rtl) {
>+  transform-origin: right 20px;
>+}

>+panel[arrowposition="start_after"]:-moz-locale-dir(ltr) {
>   transform-origin: right calc(100% - 20px);
> }

>+panel[arrowposition="end_after"]:-moz-locale-dir(rtl) {
>+  transform-origin: right calc(100% - 20px);
> }

>+panel[arrowposition="end_before"]:-moz-locale-dir(ltr) {
>   transform-origin: left 20px;
> }

>+panel[arrowposition="start_before"]:-moz-locale-dir(rtl) {
>+  transform-origin: left 20px;
> }

>+panel[arrowposition="end_after"]:-moz-locale-dir(ltr) {
>   transform-origin: left calc(100% - 20px);
> }

>+panel[arrowposition="start_after"]:-moz-locale-dir(rtl) {
>+  transform-origin: left calc(100% - 20px);
>+}

These rules should be merged in pairs of two. (I reordered them to make my point.)

Comment 5

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed7a9a1038c5

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Backed out for being the likely cause of Windows mochitest-5 permafail.
https://hg.mozilla.org/integration/mozilla-inbound/rev/8643f8fb07a2

https://tbpl.mozilla.org/php/getParsedLog.php?id=42375547&tree=Mozilla-Inbound

Comment 7

[Neil Deakin]

Attachment: Fix transition handling

The change here for some reason changed the transition detection so a popup from an earlier test doesn't close properly. Fortunately, we have a proper way to detect whether a transition exists now. This patch just changes to use nsLayoutUtils::HasCurrentAnimations after a flush. The conditions are moved around to ensure that the flush is done is less circumstances.

Comment 8

[Neil Deakin]

Try build now passes: https://tbpl.mozilla.org/?tree=Try&rev=b2b958947fa3

Comment 9

[Neil Deakin]

Comment on attachment 8447688 [details] [diff] [review]
Fix transition handling

Maybe someone else can review.

Comment 10

[[e-mail]]

Comment on attachment 8447688 [details] [diff] [review]
Fix transition handling

I'm starting to catch up with reviews; sorry for the delay.

The patch context is sadly insufficient to include the start of the comment, where you assume that it has a closing transition. Now that you're actually checking rather than assuming, it would be nice of the comment could be fixed too :-)

Comment 11

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1080f2330d51
https://hg.mozilla.org/integration/mozilla-inbound/rev/c93d64f6a76a

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1080f2330d51
https://hg.mozilla.org/mozilla-central/rev/c93d64f6a76a

Comment 13

[Marco Mucci [:MarcoM]]

Hi Florin, can a QA contact be assigned to this bug for verification.

Comment 14

[Roman Müller]

I don't understand why this is being fixed while same issue reported for regular Firefox browser with menu button on the left side was marked as wontfix.

https://bugzilla.mozilla.org/show_bug.cgi?id=1032186

Comment 15

[Neil Deakin]

This bug affects people using the default build. That other bug does not.

That said, I think bug 1032186 would be fixed by 974389.

Comment 16

[Marco Mucci [:MarcoM]]

Hi Cornel, following up to see if this bug can be verified by Monday which is the end of the Desktop iteration.

Comment 17

[Cornel Ionce [:noni] [Hubs QA]]

Thested on latest Firefox Nightly 33.0a1, build ID: 20140716030202 using the RTL builds (Ar, Fa, He, Ug and Ur) on the following OSs:
1. Windows 7 64bit - Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
2. Ubuntu 14.04 32bit - Mozilla/5.0 (X11; Linux i686; rv:33.0) Gecko/20100101 Firefox/33.0
3. Mac OS X 10.9.4 - Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:33.0) Gecko/20100101 Firefox/33.0

The arrow-panel animations have their origin on their right side. Marking issue verified.

Comment 18

[Neil Deakin]

Attachment: Minor fix to restore closing animation

Philipp found an issue here where the closing animation isn't correct from what it used to be. It shouldn't resize closed. He tested this patch and is ok with the result.

Comment 19

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d323591130d0

Comment 20

[Neil Deakin]

This fixes up the comment as indicated by comment 10.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7c9ad40d8183

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d323591130d0
https://hg.mozilla.org/mozilla-central/rev/7c9ad40d8183