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

VERIFIED FIXED

Status

defect
VERIFIED FIXED
8 years ago
3 years ago

People

(Reporter: mitcho, Assigned: mitcho)

Tracking

Dependency tree / graph
Bug Flags:
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [visual][polish])

Attachments

(2 attachments, 1 obsolete attachment)

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.
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.
Assignee: nobody → mitcho
Status: NEW → ASSIGNED
Attachment #514036 - Flags: feedback?(tim.taubert)
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.
Attachment #514036 - Flags: feedback?(tim.taubert) → feedback+
Comment on attachment 514036 [details] [diff] [review]
A version of the bug 633074 patch which also fixes this bug

Thanks Tim.
Attachment #514036 - Flags: review?(ian)
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?
Attachment #514036 - Flags: review?(ian) → review+
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?
(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 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 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).
Attachment #514036 - Flags: approval2.0?
Comment on attachment 514036 [details] [diff] [review]
A version of the bug 633074 patch which also fixes this bug

a=beltzner
Attachment #514036 - Flags: approval2.0? → approval2.0+
Patch for checkin. No test included; requesting litmus.
Attachment #514036 - Attachment is obsolete: true
Flags: in-litmus?
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/4e6809c053cd
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Version: unspecified → Trunk
Blocks: 634574

Comment 12

8 years ago
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.
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus?(vlad.maniac)
Flags: in-litmus?(vlad.maniac) → in-litmus+
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.