[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