635737 - Zoom animation frame is rotated when we zoom out of a non-top stacked tab

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Key frame from a screen capture

STR:
1. Stacked tabs A, B, C, ...
2. Zoom into tab A.
3. Switch to tab B or C...
4. Zoom out into Panorama.

Expected: normal zoom animation
Actual: the zoom frame is rotated, and does a little dance to become the top child after the animation.

Comment 1

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: A version of the bug 633074 patch which also fixes this bug

I had originally hoped that the patch to bug 633074 would fix this, but it does not... however, a more general version of the bug 633074 patch *does*. The issue is, the patch there only applies if the tab we're zooming out of is "hidden", i.e. one of the lower tabs that are hidden in the stack. What we want to do is to make all the other tabs also get the same treatment: make them topChild before we do any animation.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 514036 [details] [diff] [review]
A version of the bug 633074 patch which also fixes this bug

Looks good! Let's combine those two patches.

Comment 3

[Michael Yoshitaka Erlewine [:mitcho]]

Comment on attachment 514036 [details] [diff] [review]
A version of the bug 633074 patch which also fixes this bug

Thanks Tim.

Comment 4

[Ian Gilman [:iangilman]]

Comment on attachment 514036 [details] [diff] [review]
A version of the bug 633074 patch which also fixes this bug

Right on. 

Is this testable? Should we bother?

Comment 5

[Michael Yoshitaka Erlewine [:mitcho]]

The STR here wouldn't really be testable deterministically with mochi... I would suggest adding a litmust test.

Tim, how about the 633074 STR? You think we could test that?

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

(In reply to comment #5)
> Tim, how about the 633074 STR? You think we could test that?

I think we could test that but this would just roughly check whether the tabItem is hidden or not. And I tried checking zoom states which is really no fun :/

Comment 7

[Michael Yoshitaka Erlewine [:mitcho]]

Comment on attachment 514036 [details] [diff] [review]
A version of the bug 633074 patch which also fixes this bug

Thanks Tim. Let's ask for approval and see what the approvers say, then.

But before that, pushed to try: http://tbpl.mozilla.org/?tree=MozillaTry&rev=b73cefda8ed7

Comment 8

[Michael Yoshitaka Erlewine [:mitcho]]

Comment on attachment 514036 [details] [diff] [review]
A version of the bug 633074 patch which also fixes this bug

Passed try, modulo three known intermittents.

Note to approvers: This (trivial, lightweight) patch fixes this bug as well as bug 633074. Both are visual flaws in how the zoom animation works for a tab in a stack. As such, writing deterministic tests for these behaviors could be quite tricky, as we discussed above, so this is submitted without mochitests. Litmust tests may be appropriate. r+ Ian, passed try (link above).

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 514036 [details] [diff] [review]
A version of the bug 633074 patch which also fixes this bug

a=beltzner

Comment 10

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch for checkin

Patch for checkin. No test included; requesting litmus.

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/mozilla-central/rev/4e6809c053cd

Comment 12

[George Carstoiu]

Mozilla/5.0 (X11; Linux i686; rv:2.0b13pre) Gecko/20110224 Firefox/4.0b13pre

Verified issue and it's no longer present with the latest build.

Comment 13

[Maniac Vlad Florin (:vladmaniac)]

Litmus test created 

https://litmus.mozilla.org/show_test.cgi?id=15180