[Webkit-unassigned] [Bug 10652] Need SVGFontFace*Element(s) DOM objects

[bugzilla-daemon at webkit.org]

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


darin at apple.com changed:

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




------- Comment #6 from darin at apple.com  2007-10-05 14:38 PDT -------
(From update of attachment 16548)
+ This file is part of the WebKit project

We don't normally include that in headers. I'd prefer to just omit it.

+    class SVGDefinitionSrcElement : public SVGElement
+{
+public:
+    SVGDefinitionSrcElement(const QualifiedName&, Document*);
+    virtual ~SVGDefinitionSrcElement();
+    
+    virtual void childrenChanged();
+};

This looks indented wrong. Brace goes on same line as function definition.

+namespace WebCore
+{

Brace goes on same line as namespace.

+    virtual ~SVGDefinitionSrcElement();

There's no need to declare and define the destructor explicitly in a case like
this where the destructor is already virtual, unless there's something about
the constructor that you can't compile given just the header (like a data
member that is a RefPtr for a type that's forward declared). For simple classes
it's far better to remove it.

+#define SVGDefinitionSrcElement_h
+#if ENABLE(SVG)

Normally we include "Platform.h" before using macros like ENABLE.

+// Defined in CSSGrammar.y, but not in any header, so just declare it here for
now.
+int getPropertyID(const char* str, int len);

Lets not replicate this hack. If we need to use this in more places, then lets
fix the problem.

+#if 0
+    if (definitionSrc)
+        m_styleDeclaration->setProperty(CSS_PROP_DEFINITION_SRC,
definitionSrc->getAttribute(XLinkNames::hrefAttr), false);
+#endif

Please don't check in code with #if 0; if you must please include a comment.

I think we need some sort of deferral of the work in rebuildFontFace -- if you
do a sequence of DOM modifications, the SVGFontFaceElement will keep rebuilding
itself over and over again. Maybe we can make the style declaration get rebuild
as part of recalcStyle?


-- 
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.