[Webkit-unassigned] [Bug 12499] External <use> xlink:href references do not work

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat May 24 23:01:34 PDT 2008


http://bugs.webkit.org/show_bug.cgi?id=12499


darin at apple.com changed:

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




------- Comment #10 from darin at apple.com  2008-05-24 23:01 PDT -------
(From update of attachment 21102)
+    This file is part of the KDE libraries

I don't think it's helpful to include this in WebKit source files.

 #include "CachedXSLStyleSheet.h"
+#if ENABLE(SVG)
+#include "CachedSVGDocument.h"
+#endif
 #include "CString.h"

We normally put includes that require an #if in a separate paragraph, after all
the unconditional includes.

 class CachedXSLStyleSheet;
+#if ENABLE(SVG)
+class CachedSVGDocument;
+#endif
 class Document;

Same with class forward declarations.

+    CachedSVGDocument* requestSVGDocument(const String &url);

Please put the & next to the typename.

 #include "XLinkNames.h"
+#include "DocLoader.h"
+#include "CachedSVGDocument.h"

Please keep includes sorted alphabetically.

+    , m_cachedDoc(0)

Why abbreviate? Why not "m_cachedDocument"?

+    if (url.find('#') > -1) { // format is #target
+        unsigned int end = url.find('#');

It doesn't make sense to call find twice here. Since this is URL parsing, I
think this should be done with a helper function in KURL.h. It would go along
with the others, like equalIgnoringRef and protocolIs. No need to do that in
this patch, but a good idea in the end. I really don't think that getDocUrl is
a good name for this function. The abbreviation is unclear, for one thing.

+    if (docUrl.isEmpty())
+        return document();
+    else

We normally don't do else after return.

+    const Document* doc = referencedDocument();

Why const?

You need a test case for this new feature! I'm setting review- because of the
lack of a test case mainly.


-- 
Configure bugmail: http://bugs.webkit.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