Dirkjan Ochtman (:djc) Reporter
	Description • 6 years ago

See the attached screenshots from my Nexus 4 (Android 5).

This regressed somewhere in the past few weeks.

	Ryan Hunt [:rhunt]
	Comment 4 • 6 years ago

Is this always reproducible or just intermittently? Are there any special steps to reproduce it besides scrolling on a twitter timeline?

If possible do you have a good build date so we could bisect when the regression was introduced?

	Dirkjan Ochtman (:djc) Reporter
	Comment 5 • 6 years ago

It's not always reproducible. It seems it might be related to images or ads in tweets.

Sorry, I don't have an exact build date, I would start with something like 8-12 weeks ago.

	Ryan Hunt [:rhunt]
	Comment 6 • 6 years ago

Jamie, have you seen anything like this before? Or are you able to reproduce it?

	Jamie Nicol [:jnicol] Assignee
	Comment 7 • 6 years ago

Hmm, I can reproduce this very intermittently, going to be hard to get a regression range! I'll look in to it further.

	Julien Wajsberg [:julienw]
	Comment 9 • 6 years ago

I see this on Beta, setting the flags appropriately.

	Jamie Nicol [:jnicol] Assignee
	Comment 10 • 6 years ago

There's a few issues on twitter currently, so let's make this bug solely about content being rendered in the wrong place, and remaining there until another invalidation or scroll occurs. I've found the regression range: bug 1477693. This can be worked around by setting layout.display-list.flatten-transform=false. Note this is a separate issue than the sticky header bar flickering. Also, there appears to be yet another issue with content briefly flashing in the wrong place, which has been around for a long time, which I'll file a follow up for.

After discussing this with Miko, I think we understand this and have a fix. In ComputeGeometryChangeForItem, we apply a shift to some rects and geometries to account for a change in scroll offset. With transform flattening, the shift is taken care of by the root transform in the item's TransformClipTree, so we no longer need to apply it manually. One of the invalidation code branches combines the old area and new area of the invalidated frame. Prior to bug 1477693, this combined area was transformed with the new transform, which takes account of the shift. Post bug 1477693, the old area is transformed by the old transform, which does not take in account the new shift. We must therefore apply the shift before combining it with the new area.

Miko can correct me if the above is wrong. Patch on try currently.

	Mike Taylor [:miketaylr]
	Comment 11 • 6 years ago

Any progress here? 

Also, P3 seems low for twitter.com.

	Sorina Florean [:sflorean]
	Comment 12 • 6 years ago

We tried to reproduce this issue also but unfortunately, we don't have Nexus 4 and we tried with Nexus 5(Android 6.0.1) and Xiaomi Mi4i(Android 5.0.2) and we couldn't reproduce it. We will investigate more.

	Julien Wajsberg [:julienw]
	Comment 13 • 6 years ago

FWIW I still see this on latest nightly.

	Julien Wajsberg [:julienw]
	Comment 14 • 6 years ago

This is how I reproduce it:
1. scroll down twitter with a momentum (with a "swipe down" gesture)
2. when it's white because of the overscroll, tap once so that the scroll stops
3. wait until the overscroll stops

	Pascal Chevrel:pascalc (PTO until August 21)
	Comment 15 • 6 years ago

  (In reply to Jamie Nicol [:jnicol] from comment #10)
> There's a few issues on twitter currently, so let's make this bug solely
> about content being rendered in the wrong place, and remaining there until
> another invalidation or scroll occurs. I've found the regression range: bug
> 1477693. This can be worked around by setting
> layout.display-list.flatten-transform=false.

Miko, Jamie, would it be possible to set the above pref to false for 63? What would be the trade off?

	Jamie Nicol [:jnicol] Assignee
	Comment 16 • 6 years ago

Sorry for my inactivity on this bug. I have a patch which looks good on try, I'll get it landed today.

The trade-off would be a performance regression, this patch is simple so it'll be better to land it instead.

	Pascal Chevrel:pascalc (PTO until August 21)
	Comment 17 • 6 years ago

(In reply to Jamie Nicol [:jnicol] from comment #16)
> Sorry for my inactivity on this bug. I have a patch which looks good on try,
> I'll get it landed today.
> 
> The trade-off would be a performance regression, this patch is simple so
> it'll be better to land it instead.

Could  your patch be ready to be uplifted this week before we enter RC week?

	Jamie Nicol [:jnicol] Assignee
	Comment 18 • 6 years ago

Yes, easily.

	Jamie Nicol [:jnicol] Assignee
	Comment 19 • 6 years ago

When calculating which regions of a layer to invalidate we usually
apply a shift to the area to account for changes in scroll offset. For
items within flattened transforms we do not do this, because the
transform itself includes the scroll offset. However, when calculating
the old area of an invalidated frame, we use the old transform. This
includes the previous scroll offset rather than the current, so we
must therefore still apply the shift.

Not doing so was causing the incorrect region to be invalidated, and
content to be rendered at the wrong location.

	Pascal Chevrel:pascalc (PTO until August 21)
	Comment 20 • 6 years ago

Tracking for 63 as it affects a top site and we have a patch in view.

	Pulsebot
	Comment 21 • 6 years ago

Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cf2d65b646d1
Apply shift when calculating old area of invalidated frame r=miko

	Raul Gurzau (:RaulG)
	Comment 22 • 6 years ago
bugherder

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

	Pascal Chevrel:pascalc (PTO until August 21)
	Comment 23 • 6 years ago

Jamie, could you request uplift to beta? Thanks

	Jamie Nicol [:jnicol] Assignee
	Comment 24 • 6 years ago

Comment on attachment 9015509 [details]
Bug 1490789 - Apply shift when calculating old area of invalidated frame

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1477693

User impact if declined: Content drawn in wrong position on some websites, including twitter

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Minor change. Ensures we invalidate the correct area.

String changes made/needed:

	Jamie Nicol [:jnicol] Assignee
	Comment 25 • 6 years ago

I meant to say no to needs manual test, I have verified it myself. But in any case STR are scrolling quickly on twitter then stopping it. Or alternatively pull down to refresh, then as it is loading quickly tap on a tweet then the phone's back button.

	Pascal Chevrel:pascalc (PTO until August 21)
	Comment 26 • 6 years ago

Comment on attachment 9015509 [details]
Bug 1490789 - Apply shift when calculating old area of invalidated frame

Fix for a visible rendering issue on Android affecting at least one major site, approved for uplift to our last 63 beta. Thanks.

	Ryan VanderMeulen [:RyanVM]
	Comment 27 • 6 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/c4fd0c5a2d5a

	Ioana Chiorean
	Comment 28 • 6 years ago

I tried the mentioned steps (scrolling quickly) but did not see anything out of place (neither text nor images) with latest 2 builds of Nightly 64.0a1(2018.10.11-12) both in browser and PWA on several devices: 
* Motorola Nexus 6(Android 7.1.1) 
* Nexus 9(Android 6.0.1)
* Pixel 2 (Android 9)
* Samsung S8 Edge (Android 8.0.0)

I did see  a slightly delay in showing the pictures content (like 2sec) like in the second uploaded picture (displays a rectangle with the color major from the picture) BUT the image is aligned and not overlapping lines nor text. That is twitter intended behavior. 
We will also check this at the next beta/tue

	Bogdan Surd, QA [:bsurd, NI]
	Comment 29 • 6 years ago

Devices:
 - OnePlus 5T (Android 8.1.0)
 - Huawei P9 Lite (Android 6)

Tried in the latest beta (63.0b15), everything worked as expected in both normal browsing as well as PWA. Marking as verified.