[Webkit-unassigned] [Bug 6427] <tref> element not implemented

bugzilla-daemon at opendarwin.org bugzilla-daemon at opendarwin.org
Mon Apr 17 01:37:14 PDT 2006


http://bugzilla.opendarwin.org/show_bug.cgi?id=6427


macdome at opendarwin.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #7757|review?                     |review-
               Flag|                            |




------- Comment #2 from macdome at opendarwin.org  2006-04-17 01:37 PDT -------
(From update of attachment 7757)
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.


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list