Closed Bug 1124610 Opened 5 years ago Closed 5 years ago

FF36: Incorrect repainting after scrolling down and up.

Categories

(Core :: Graphics, defect)

36 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla38
Tracking Status
firefox35 --- unaffected
firefox36 + verified
firefox37 + fixed
firefox38 + verified

People

(Reporter: pjemen, Assigned: seth)

References

Details

(Keywords: regression, Whiteboard: gfx-noted)

Attachments

(3 files)

Attached image ff37beta-buildbot.png
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150120155007

Steps to reproduce:

win7x64.
FF36.
For example visit page http://winbot.zope.org/waterfall
Scroll down and up.




Actual results:

Page is incorrectly repainted.


Expected results:

Correctly repainted.
[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Got as far as we can go bisecting nightlies...
Last good revision: 74edfbf0e6a3
First bad revision: ced1402861b8
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=74edfbf0e6a3&tochange=ced1402861b8

Last good revision: 4631a7474d8a
First bad revision: bb51e79ff4fa
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4631a7474d8a&tochange=bb51e79ff4fa
Status: UNCONFIRMED → NEW
Component: Untriaged → General
Ever confirmed: true
Flags: needinfo?(seth)
Keywords: regression
Product: Firefox → Core
I can reproduce this bug using the URL in comment 0 on OSX 10.10.1.

Milan - Can you please investigate to see if this is a gfx issue?
Component: General → Graphics
Flags: needinfo?(milan)
Not seeing this issue on 10.9.5, but I did see a black rectangle at one point.  Given the regression range - Seth, any ideas?  SVG?
Flags: needinfo?(milan)
Whiteboard: gfx-noted
I can reproduce as well on 10.10. The background behind the header - where the word "Waterfall" is in a larger font - is initially white, but after scrolling down and back up it becomes a dark gray. Scrolling up and down again fixes it.
Flags: needinfo?(seth)
Further investigation reveals:

- If I bring up the dev tools and inspect the header, it does not get repainted correctly, even in the areas highlighted by the dev tools inspector.

- If I use the dev tools to delete elements from outside the affected area - like, say, the table - they disappear. However, if I delete elements from inside the affected area - for example, the <h1> element containing the word "Waterfall" - the affected area does not get repainted. The dev tools act as if the <h1> element is gone, but I can still see the word "Waterfall" on the screen.
Looks like body has a repeat-x background image on it - 'bg_gradient.jpg' - which is 120px high, probably approximately the height of the affected area. I'd guess something's going wrong with that image.

Looking at that pushlog, nothing immediately jumps out at me as a logical explanation. The first thing to do is bisect within that pushlog and figure out exactly which commit broke it.
Flags: needinfo?(seth)
I'll build beta overnight and will try to bisect this to the exact commit tomorrow.
Assignee: nobody → seth
I've bisected this to this commit:

5ff89c5a8393	Seth Fowler — Bug 1102617 - Replace imgIContainer::FrameIsOpaque with imgIContainer::IsOpaque. r=tn

Quite possibly relevant is this message which I noticed we print for this page:

Corrupt JPEG data: 1 extraneous bytes before marker 0xc4
Blocks: 1102617
Alright, I just debugged this, and the problem is straightforward. When a decoder bails out of decoding due to an error, it may not have written to its entire frame. The areas of the frame that it hasn't written to are zero-initialized, and since the default frame format is B8G8R8A8, we end up with transparency that we have failed to announce via PostHasTransparency(), which is not allowed. This causes the layout and painting code to believe that the corrupt image on this page is opaque when it isn't, and the result is bad rendering.

The fix is to call PostHasTransparency() if we're completing decoding due to an error.
Flags: needinfo?(seth)
Here's the patch.
Attachment #8558443 - Flags: review?(jmuizelaar)
Attachment #8558443 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/40d3b44fb02c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Here's a backport of this patch to 36, suitable for uplift.
Comment on attachment 8560865 [details] [diff] [review]
Call PostHasTransparency for corrupt images that we treat as usable. (FF36 Backport)

Approval Request Comment
[Feature/regressing bug #]: 1102617
[User impact if declined]: Painting artifacts while scrolling or possibly in other situations, like during animations.
[Describe test coverage new/current, TreeHerder]: On m-c.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.
Attachment #8560865 - Flags: approval-mozilla-beta?
Attachment #8560865 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Seth, what is the plan about 37/aurora? Thanks
Flags: needinfo?(seth)
QA Whiteboard: [good first verify]
I've just updated to FF36 Beta 8 (BUILDID:20150209164123) and it problem is fixed.
(In reply to palo misik from comment #18)
> I've just updated to FF36 Beta 8 (BUILDID:20150209164123) and it problem is
> fixed.

Thank you! Marking as Verified on Firefox 36.
Managed to reproduce the bug on an affected nightly build from 2015-01-22, using Windows 8.1 (x64) with the link from Comment 0.

This is now verified fixed on Nightly 38.0a1 (2015-02-19) using Windows 8.1 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
Seth - Firefox 37 is still affected. Does this require a branch specific patch for Aurora? Can you please get the patch in shape to land on Aurora and submit an uplift request (preferably before the merge on Monday)?
Comment on attachment 8558443 [details] [diff] [review]
Call PostHasTransparency for corrupt images that we treat as usable

Approval Request Comment
[Feature/regressing bug #]: Bug 1102617
[User impact if declined]: Rendering glitches on pages with corrupt images.
[Describe test coverage new/current, TreeHerder]: On m-c for some time.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.
Flags: needinfo?(seth)
Attachment #8558443 - Flags: approval-mozilla-aurora?
Comment on attachment 8558443 [details] [diff] [review]
Call PostHasTransparency for corrupt images that we treat as usable

Verbal approval previously given as this had already landed on 36 and 38. Aurora+
Attachment #8558443 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.