[webkit-reviews] review denied: [Bug 10380] tspans don't support
styles : [Attachment 10813] styled tspan patch #4 million and one
bugzilla-request-daemon at opendarwin.org
bugzilla-request-daemon at opendarwin.org
Wed Sep 27 20:01:44 PDT 2006
Eric Seidel <macdome at opendarwin.org> has denied Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 10380: tspans don't support styles
http://bugzilla.opendarwin.org/show_bug.cgi?id=10380
Attachment 10813: styled tspan patch #4 million and one
http://bugzilla.opendarwin.org/attachment.cgi?id=10813&action=edit
------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
marking this for review so others will take a peak.
I've looked through, there are definitely some style issues left to resolve.
I'm also not certain the placement of the filter/clip/mask code is correct. I
don't think we want to filter the individual boxes, unless there is only one
box per tspan/text (since that would cause blurs to be off).
Also, SVG_SUPPORT is missing from dom/Text.cpp and possibly other core files.
Some example style issues:
+RenderSVGTSpan::RenderSVGTSpan(Node *n):RenderInline(n) {}
Node* n not Node *n
also generally : RenderInline is on the next line, indented
and in this case { } would be on separate lines after that
+ InlineFlowBox* initFlow=firstLineBox();
generally we use spaces before and after =
This could benefit from argument names:
+ virtual InlineBox* createInlineBox(bool, bool, bool isOnlyRun = false);
+void SVGInlineTextBox::paint(RenderObject::PaintInfo& paintInfo, int tx, int
ty){
{ should go on a separate line.
Nice work olliej. I can't wait to see this land.
More information about the webkit-reviews
mailing list