Summary: | Directly-composited animated GIFs never resume once scrolled offscreen | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | Layout and Rendering | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, dbates, esprehn+autocc, glenn, koivisto, kondapallykalyan, simon.fraser, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Chris Dumez
2016-01-06 17:01:08 PST
Created attachment 268461 [details]
Patch
Comment on attachment 268461 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268461&action=review > Source/WebCore/ChangeLog:14 > + RenderBoxModelObject::contentChanched(ImageChanged) in addition to contentChanched => contentChanged > Source/WebCore/ChangeLog:26 > + Call RenderBoxModelObject::contentChanched(ImageChanged) in addition to Ditto > Source/WebCore/ChangeLog:38 > + to cover this bug as our rendering code believed it has unpaused the Nit: "has unpaused" => "had resumed" > Source/WebCore/rendering/RenderElement.cpp:1519 > + // Calling repaint() does not suffice to restart the animation in the case of > + // directly-composited animated GIFs so we need to do this. Maybe a better way to write this is: Mark the image as changed to resume directly-composited animated GIFs. OR For directly-composited animated GIFs it does not suffice to call repaint() to resume animation. We need to mark the image as changed. > LayoutTests/fast/images/composited-animated-gif-outside-viewport.html:7 > +<img id="testImage" src="resources/animated.gif" style="-webkit-transform: translatez(0);" /> Please remove the trailing slash character as the img tag does not have a closing tag among other reasons. > LayoutTests/fast/images/composited-animated-gif-outside-viewport.html:36 > +function scrollDown() { Nit: The position of the opening curly brace for this function differs from the position of the opening curly brace used in other functions in this file (e.g. scrollUp). I suggest we place the opening brace on its own line. Regardless, we should pick one style and stick with it throughout this file. > LayoutTests/fast/images/composited-animated-gif-outside-viewport.html:42 > +function runTest() { Ditto. Comment on attachment 268461 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268461&action=review > Source/WebCore/ChangeLog:10 > + Directly-composited animated GIFs would never resume once scrolled > + offscreen. This is because calling repaint() in this case would not Would this also affect SVG images? If so, we should add a test for this. (In reply to comment #4) > Comment on attachment 268461 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=268461&action=review > > > Source/WebCore/ChangeLog:10 > > + Directly-composited animated GIFs would never resume once scrolled > > + offscreen. This is because calling repaint() in this case would not > > Would this also affect SVG images? If so, we should add a test for this. Tim Horton mentioned on IRC today that that this does not affect SVG images as they are not directly composited. Created attachment 268468 [details]
Patch
Comment on attachment 268468 [details] Patch Clearing flags on attachment: 268468 Committed r194706: <http://trac.webkit.org/changeset/194706> All reviewed patches have been landed. Closing bug. |