[webkit-reviews] review denied: [Bug 6427] <tref> element not implemented : [Attachment 7757] Possible fix.

[bugzilla-request-daemon at opendarwin.org]

Eric Seidel <macdome at opendarwin.org> has denied Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 6427: <tref> element not implemented
http://bugzilla.opendarwin.org/show_bug.cgi?id=6427

Attachment 7757: Possible fix.
http://bugzilla.opendarwin.org/attachment.cgi?id=7757&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
A couple comments:

1.  This violates the style guidelines:
+    if (SVGURIReference::parseMappedAttribute(attr)) return;
(the return should be on its own line)

+    DeprecatedString ref = String(href()->baseVal()).deprecatedString();

is unnecessarily complicated.  If you really need a deprecatedString, I believe
you can ask the AtomicString directly.

If you're going to ignore the exception you might call this variable "ignored"
+	 ExceptionCode ec = 0;
unless there is already a version of createTextNode which ignores exceptions.

New code should have the * close to the type not the variable name.

Generally we try not to use closeRenderer, but rather respond to dynamic
updates.  In this case we should update the text children whenever the href
property changes.  Probably a call to setTextContent would work well for that.

Are you sure you should return RenderInline and not RenderSVGText?  I'd have to
go stare at both to decide myself.

I think there are enough minor issues here to prevent landing.