[Webkit-unassigned] [Bug 11625] Investigate possibility to share code between HTMLStyleElement and SVGStyleElement

[bugzilla-daemon at webkit.org]

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





------- Comment #6 from rwlbuis at gmail.com  2006-11-19 05:31 PDT -------
Hi Darin,

(In reply to comment #5)
> (From update of attachment 11550 [edit])
> The Document.cpp change is great, but it seems like a separate fix to me --
> it's independent of the inheritance change.

Right. At first I wanted to be able to cast both SVGStyle and HTMLStyle to a
common base class and use that, and in the end not needing SVG_SUPPORT tests. I
don't think that is doable anymore since svg needs slightly different behaviour
here than html. Also it would require a dynamic_cast.

> THe new base class seems OK. It's good not to have two entire copies of the
> code, even though we end up using multiple inheritance.
> 
> +    , StyleElement()
> 
> You should not need to explicitly invoke the base class's default constructor
> like this.

Fixed.

> +//fprintf(stderr, "isLoading %d\n", m_loading);
> 
> Please don't land these.

Heh, I need to check better before submitting. Fixed.

> Why all the changes from AtomicString to String? When possible, it's great to
> have functions take AtomicString parameters, in case you're already calling
> with an atomic string -- saves a hash table lookup. And it's great to have
> const AtomicString& return types instead of String in cases where that works --
> much more efficient, but you can't always do it.

I tried it and I have an upcoming patch for it. Indeed the code seems much
better that way.
Cheers,

Rob.


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