904616 - Correct sizes for hidden preload browsers are never applied

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Someone (me) broke the efforts from bug 875257 with bug 881590. Which means that the preloading browsers end up not having the right sizes and we have reflows after docShells have been swapped :(

Nominating for tracking because fixing this should be trivial and we certainly don't want a slow tabopen animation.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Ensure that sizes for preload browsers are applied correctly

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 789596 [details] [diff] [review]
Ensure that sizes for preload browsers are applied correctly

Review of attachment 789596 [details] [diff] [review]:
-----------------------------------------------------------------

Wasn't there supposed to be a test that made sure we didn't regress this?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Jared Wein [:jaws] from comment #2)
> Wasn't there supposed to be a test that made sure we didn't regress this?

Yeah, that didn't quite land yet...

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/cb1b1f0d16ff

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cb1b1f0d16ff

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Ensure that sizes for preload browsers are applied correctly (for beta and aurora)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 881590
User impact if declined: Slow and choppy tabopen animation.
Testing completed (on m-c, etc.): Landed on m-c.
Risk to taking this patch (and alternatives if risky): Low risk.
String or IDL/UUID changes made by this patch: None.

Asking for Beta and Aurora approval as this is a really simple fix that vastly improves the tabopen animation, especially on slower machines.

Comment 7

[bhavana bajaj [:bajaj]]

(In reply to Tim Taubert [:ttaubert] from comment #6)
> Created attachment 790661 [details] [diff] [review]
> Ensure that sizes for preload browsers are applied correctly (for beta and
> aurora)
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): bug 881590
> User impact if declined: Slow and choppy tabopen animation.
> Testing completed (on m-c, etc.): Landed on m-c.
> Risk to taking this patch (and alternatives if risky): Low risk.
> String or IDL/UUID changes made by this patch: None.
> 
> Asking for Beta and Aurora approval as this is a really simple fix that
> vastly improves the tabopen animation, especially on slower machines.

Tim, is there a test case for this patch ? Also do we need any help from QA with verification by performing any manual testing for any test cases you may have in mind ?

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

(In reply to bhavana bajaj [:bajaj] from comment #7)
> Tim, is there a test case for this patch ? Also do we need any help from QA
> with verification by performing any manual testing for any test cases you
> may have in mind ?

That's unfortunately bug 876218. I started to work on this a few moons ago but was held up by try issues, iirc. Verifying this is a little tricky, you would need to write some code for it and run that. I could do that, though once it landed on beta/aurora. Or is that maybe not the same as somebody doing it from the QA team? :)

Comment 9

[bhavana bajaj [:bajaj]]

(In reply to Tim Taubert [:ttaubert] from comment #8)
> (In reply to bhavana bajaj [:bajaj] from comment #7)
> > Tim, is there a test case for this patch ? Also do we need any help from QA
> > with verification by performing any manual testing for any test cases you
> > may have in mind ?
> 
> That's unfortunately bug 876218. I started to work on this a few moons ago
> but was held up by try issues, iirc. Verifying this is a little tricky, you
> would need to write some code for it and run that. I could do that, though
> once it landed on beta/aurora. 

That will be very helpful :) thanks !

>Or is that maybe not the same as somebody
> doing it from the QA team? :)

Comment 10

[bhavana bajaj [:bajaj]]

Comment on attachment 790661 [details] [diff] [review]
Ensure that sizes for preload browsers are applied correctly (for beta and aurora)

Approving given this is low risk FX25 regression and we are still in early beta's.Tim to help with verification once this lands.

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3b956b39bac4
https://hg.mozilla.org/releases/mozilla-beta/rev/cc28055780cd

Comment 12

[Manuela Muntean [Away]]

(In reply to Tim Taubert [:ttaubert] from comment #8)
> Verifying this is a little tricky, you would need to write some code for it and run that. I could do >that, though once it landed on beta/aurora. Or is that maybe not the same as somebody
> doing it from the QA team? :)

Hi Tim! Are there any news/updates regarding this? Is there anything QA can help with here? Thanks!

Comment 13

[Tracy Walker [:tracy]]

Visually this looks smooth and correct on 24beta10 and Aurora and Nightly builds of 20130913.

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

Confirmed that an old beta version had the wrong sizes. After updating the preload browsers have the correct dimensions.