Bug 845147 is a regression from bug 689623 that hasn't yet been mitigated. This bug is to decide if we disable bug 689623 on beta (by flipping a pref). We should do this at the start of the beta period if we do it (which just started now).

Try had some problems today, here is a new try run https://tbpl.mozilla.org/?tree=Try&rev=60732faf7b2c

A patch from bug 853564 landed on Aurora (now beta) that was depending on only visible images being tracked (ie bug 689623 being enabled), see 853564 comment 3. So we'd have to do something about that.

Tumblr Archives and Pinterest (presumably affected) would warrant disabling, yes. We reproduced in triage and it's bad.

I talked to Joe about this. We decided the the least risky approach is to land the better (but riskier) patch from bug 853564 (the one that doesn't rely on bug 689623) along with any followup fixes from that bug. That will allow us to disable 689623 on beta without introducing any regressions and not subjecting us to any unnecessary risk.

Comment on attachment 749513 [details] [diff] [review] patch Doing this will cause us to revert the fix to bug 792199, meaning we'll cause janky tab switches to tabs with lots of images. That's because I fixed bug 853564 in a safer way for 22. Fixing it in a way that doesn't cause janky tab switches in 22 requires us to back out the safer fix from bug 853564 and land: bug 853564, bug 868871, bug 869125, bug 871671 (at minimum). Product drivers will have to decide which they hate least, I guess: the janky scrolling, the janky tab switch, or forward-porting those patches.

(In reply to Joe Drew (:JOEDREW! \o/) from comment #6) > Product drivers will have to decide which they hate least, I guess: the > janky scrolling, the janky tab switch, or forward-porting those patches. What's the risk to backing taking the forward fixes? Once we understand that, we can make a fully informed decision. Thanks!

The risk is that forward-porting breaks things or introduces more flashing on tab switch (mainly the former). It's taken three patches to sort out the issues with the more risky fix to bug 853564, and I'm only about 75% convinced that we've found all the regressions so far.

Beta 3 is tomorrow, we should make a decision here so we can land whatever patches we decide on as soon as possible. The options: 1) janky scrolling with lots of small images (do nothing) 2) fix the janky scrolling but get janky tab switch instead (disable bug 689623 by flipping a pref) 3) fix the janky scrolling and the janky tab switch (disable bug 689623 by flipping a pref, land the better but less safe fix for bug 853564, and land the follow-ups to that: bug 868871, bug 869125, bug 871671, and maybe more) Joe said he's "only about 75% convinced that we've found all the regressions" from the less safe patch from bug 853564. Flagging Alex for needinfo as a proxy for the release drivers.

Janky tab switching when many images are present in content (return to FF18 behavior) seems like less of an issue than janky scrolling with many images (would be a new FF22 regression). We'll make sure the product team is in agreement, has a look at both b2 and b3, and we can back out this pref change if necessary in the next beta. Please help with this landing as soon as possible today. Let's make sure we also have a bug on file for the path to re-enabling bug 689623.

(In reply to Alex Keybl [:akeybl] from comment #10) > Let's make sure we > also have a bug on file for the path to re-enabling bug 689623. If I'm understanding you correctly bug 845147 is this bug.

Comment on attachment 749513 [details] [diff] [review] patch I guess we should take this on aurora as well. That will give us more testing of this configuration. We can back it out from aurora if we fix bug 845147 on aurora, but that doesn't look likely.

(In reply to Timothy Nikkel (:tn) from comment #13) > Comment on attachment 749513 [details] [diff] [review] > patch > > I guess we should take this on aurora as well. That will give us more > testing of this configuration. We can back it out from aurora if we fix bug > 845147 on aurora, but that doesn't look likely. And we have the better fix for bug 853564 on aurora so we don't have a hard trade off to make there, only beta presented us with such a conundrum.

And a followup to lower the expected number of assertions to mirror the bump when bug 689623 was landed. https://hg.mozilla.org/releases/mozilla-beta/rev/b0737c706287

This is still on NEW, but it looks fixed to me in FF 22b3 based on the STR in https://bugzilla.mozilla.org/show_bug.cgi?id=845147#c0 Any thoughts ?

I set status-firefox22: fixed, I wasn't sure what to do with a bug that only applies to branches. I'll mark it resolved with the aurora question still be to determined.

Comment on attachment 749513 [details] [diff] [review] patch Please disable on 23 (Aurora) and also in 24 for now - nominate bug 689623 for tracking if it re-lands to watch for regressions.

(In reply to lsblakk@mozilla.com [:lsblakk] from comment #18) > Please disable on 23 (Aurora) and also in 24 for now - nominate bug 689623 > for tracking if it re-lands to watch for regressions. I wasn't planning on disabling it on 24...

(In reply to Timothy Nikkel (:tn) from comment #19) > (In reply to lsblakk@mozilla.com [:lsblakk] from comment #18) > > Please disable on 23 (Aurora) and also in 24 for now - nominate bug 689623 > > for tracking if it re-lands to watch for regressions. > > I wasn't planning on disabling it on 24... Wouldn't it be best to disable until we're ready to re-enable?

And lower the assertions expectation on aurora as well https://hg.mozilla.org/releases/mozilla-aurora/rev/52db029185a4

(In reply to Alex Keybl [:akeybl] from comment #21) > (In reply to Timothy Nikkel (:tn) from comment #19) > > (In reply to lsblakk@mozilla.com [:lsblakk] from comment #18) > > > Please disable on 23 (Aurora) and also in 24 for now - nominate bug 689623 > > > for tracking if it re-lands to watch for regressions. > > > > I wasn't planning on disabling it on 24... > > Wouldn't it be best to disable until we're ready to re-enable? Then it would be getting no testing whatsoever.

Based on comments 16, 17

Will the bug be reverted because Bug 845147 is coming to be fixed.

Bug 845147 is only close to being fixed on nightly, and we've only disabled bug 689623 on aurora and beta. So bug 845147 would have to land on nightly and then be uplifted before there is anything to revert here.

Is there a bug posted to re-enable bug 689623?

Per comment 26, we disabled bug 689623 on aurora and beta /but not nightly/. That means we don't need to re-enable it.

OK Thanks, I guess this part confused me: > ... (snip) > So bug 845147 would have to land on nightly and then be uplifted before > there is anything to revert here. And it showed up as crossed out.

That's because bug 845147 was fixed since comment 26 was written.

Setting status-firefox24: wontfix meaning that we will have bug 689623 enabled on 24 (so that this bug doesn't show up in unfixed 24 bugs queries).

Why not to port the bug845147 patch to beta, so we can revert the bug872235 patch? Is there any risk?

Yes, there is some risk.