Last Comment Bug 707960 - Resolve covered region hack introduced in bug 614732
: Resolve covered region hack introduced in bug 614732
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Jonathan Watt [:jwatt]
:
: Jet Villegas (:jet)
Mentors:
Depends on: 734082 767823
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-06 09:09 PST by Jonathan Watt [:jwatt]
Modified: 2012-07-24 03:02 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (7.38 KB, patch)
2012-06-24 13:20 PDT, Jonathan Watt [:jwatt]
longsonr: review+
Details | Diff | Splinter Review

Description Jonathan Watt [:jwatt] 2011-12-06 09:09:33 PST
The covered region hack introduced in bug 614732 should be eliminated to avoid doing extra work. See bug 614732 comment 32 option #1.
Comment 1 Jonathan Watt [:jwatt] 2011-12-06 12:02:38 PST
More specifically the patch from bug 614732 comment 35 should be removed when bug 539356 is fixed.
Comment 2 Jonathan Watt [:jwatt] 2012-06-24 13:20:52 PDT
Created attachment 636181 [details] [diff] [review]
patch

Once bug 767823 is fixed, the only GetCoveredRegion caller is BoxToRect::AddBox(), via nsSVGUtils::GetOuterSVGFrameAndCoveredRegion, which is called under getBoundingClientRect/getClientRects. At that point it doesn't seem like it's worth having mCoveredRegion members on SVG leaf frames. For now we probably want to keep the GetCoveredRegion methods until we can figure out if we're going to break much content by changing them to use visual overflow rects. But I think we want getBoundingClientRect/getClientRects to act more like getBBox, in that the information is calculated on demand, rather than calculated just in case and cached the frames. This patch makes that so.
Comment 3 Robert Longson 2012-06-24 14:39:47 PDT
Comment on attachment 636181 [details] [diff] [review]
patch

> nsSVGForeignObjectFrame::GetCoveredRegion()
...
>+  // GetCanvasTM includes the x,y translation
>+  return nsSVGUtils::TransformFrameRectToOuterSVG(gfxRect(0.0, 0.0, w, h, GetCanvasTM(), PresContext());

Nit: wrap line somewhere. Other instances are already wrapped.
Comment 5 Ed Morley [:emorley] 2012-07-24 03:02:24 PDT
https://hg.mozilla.org/mozilla-central/rev/eec89f895133

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