Last Comment Bug 774095 - Make sure that we call nsSVGEffects::UpdateEffects() on frames that are added after the first reflow
: Make sure that we call nsSVGEffects::UpdateEffects() on frames that are added...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Jonathan Watt [:jwatt] (back in October - email directly if necessary)
:
Mentors:
Depends on:
Blocks: 614732
  Show dependency treegraph
 
Reported: 2012-07-15 11:19 PDT by Jonathan Watt [:jwatt] (back in October - email directly if necessary)
Modified: 2012-07-16 07:03 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.16 KB, patch)
2012-07-15 12:16 PDT, Jonathan Watt [:jwatt] (back in October - email directly if necessary)
no flags Details | Diff | Splinter Review
patch (1.16 KB, patch)
2012-07-15 14:53 PDT, Jonathan Watt [:jwatt] (back in October - email directly if necessary)
no flags Details | Diff | Splinter Review
patch (1.56 KB, patch)
2012-07-15 14:56 PDT, Jonathan Watt [:jwatt] (back in October - email directly if necessary)
dholbert: review+
Details | Diff | Splinter Review

Description Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-07-15 11:19:33 PDT
For some reason SVG frames' overflow rects sometimes don't account for filter effects. Still debugging this one.
Comment 1 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-07-15 11:59:46 PDT
This happens when frames are added after the first reflow - which apparently can be triggered by a <script> referencing an external file an the top of the doc - and then any filtered SVG frames that come after that never account for filters in their visual overflow rects. Even if the document is subsequently reflowed, or the filtered elements change, or the filter changes, the filter effect is still not accounted for in the visual overflow rect.

The reason that this happens is because nsSVGDisplayContainerFrame::InsertFrames removes NS_FRAME_FIRST_REFLOW from the kid so that nsSVGUtils::ScheduleBoundsUpdate will work when passed the kid, but then forgets to add it back afterwards. That means we never make the nsSVGEffects::UpdateEffects() call in the UpdateBounds implementations since it is guarded by a NS_FRAME_FIRST_REFLOW check.
Comment 2 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-07-15 12:16:57 PDT
Created attachment 642402 [details] [diff] [review]
patch
Comment 3 Daniel Holbert [:dholbert] 2012-07-15 13:50:11 PDT
The unconditional setting of NS_FRAME_FIRST_REFLOW here scares me a little.  Are we guaranteed that NS_FRAME_FIRST_REFLOW will be set on everything in the frame list, when this function is called?

If so, we should assert that (maybe just before we clear that bit).  And if not, we should check for it and only add it back if it was originally set.
Comment 4 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-07-15 14:53:41 PDT
Created attachment 642433 [details] [diff] [review]
patch

Ah, yeah, good point.
Comment 5 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-07-15 14:56:22 PDT
Created attachment 642434 [details] [diff] [review]
patch
Comment 6 Jonathan Watt [:jwatt] (back in October - email directly if necessary) 2012-07-15 19:54:53 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2cdd7ca5a588
Comment 7 Ed Morley [:emorley] 2012-07-16 07:03:06 PDT
https://hg.mozilla.org/mozilla-central/rev/2cdd7ca5a588

Note You need to log in before you can comment on or make changes to this bug.