277146 - [svg sr] frames

Status: RESOLVED WONTFIX

(Core :: SVG, defect)

Description

[tor]

Files:

  layout/svg/base/*

Comment 1

[Boris Zbarsky [:bzbarsky]]

In nsSVGOuterSVGFrame.cpp, the InsertFrames/AppendFrames/RemoveFrame assume that
aListName is null.  They should assert this -- it would help catch a number of
bugs in SVG frame construction.  Similar for any other SVG frames that make the
same assumption.

Comment 2

[Boris Zbarsky [:bzbarsky]]

SVG frame impls return NS_OK from addref and release.  First of all, it's the
wrong type.  Second of all, is there a reason they're overriding
nsFrame::AddRef/Release to start with?  Not overriding it would show warnings in
the cases when people are (incorrectly) addreffing and releasing SVG frames...

Comment 3

[Boris Zbarsky [:bzbarsky]]

install-svg-css.pl munges ua.css in a way that will break if we ever add an
@charset rule there.

Also, why is that being done at all?  It claims to be copying mathml, but mathml
doesn't stick itself into ua.css; it uses the catalog sheets setup...

nsISVGChildFrame.h is severely under-documented. (For example, what's the
coordinate system like for GetFrameForPoint?  What's the coordinate system of
the dirty rect for Paint?  What does InitialUpdate do?  Are the "local coords"
referred to for GetBBox the "local coords" of the SVG spec, or something else?

nsISVGContainerFrame.h could use more docs, but it's a little less vague, I guess.

nsISVGGlyphFragmentLeaf.h needs more docs on the glyph position setter and
getters (esp. about coord spaces).  Is there a reason this doesn't use
nsFloatPoint (apart from "nsFloatPoint wasn't there when this was written")? 
The documentation on IsStartOfChunk needs improvement.  What's the coordinate
system for GetAdjustedPosition?  What does GetNumberOfChars return?  The number
of glyphs, number of Unicode codepoints, number of PRUnichars?  What does
GetCharNumberOffset do?  Is this a "real" (refcounted) interface?  Or is it
implemented on frames?

nsISVGGlyphFragmentNode.h:  What does the return value of BuildGlyphFragmentTree
mean?  What do the Notify* methods do and when should they be called?  Is this a
refcounted interface?

Is nsISVGMarkable a refcounted interface?  What does GetMarkPoints actually
return?  Is caller allocated for freeing the returned objects somehow?

nsISVGOuterFrame.h: What does the boolean arg to InvalidateRegion() mean?  When
should NotifyViewportChange be called?  Will the world end if it's called by
something that's not the frame's mContent?  What do
SuspendRedraw/UnsuspendRedraw actually do?  Note bug 280363.

nsISVGSVGFrame: similar here.  Wouldn't it make sense to have nsISVGOuterFrame
inherit from this interface, since it's a strict superset?

nsISVGTextContainerFrame: documentation needed on what these meth ods do.

nsISVGTextFrame: documentation needed on when the Notify* methods are to be
called and what the other methods access (and how one could susped or unsuspend
the relevant things).  Use @see as needed.

That's that for the api comments.  Implementations...  I'm making this depend on
some bugs filed on specific issues that have already come up.  In addition to
those, should the foreign object frame actually implement a useful frametype
that would identify it as something other than "block"?

I'll try to find time to look at frame impl details later.