[webkit-reviews] review denied: [Bug 51941] refactor HTMLLinkElement to allow Link header implementation : [Attachment 78253] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 7 12:05:26 PST 2011


Alexey Proskuryakov <ap at webkit.org> has denied	review:
Bug 51941: refactor HTMLLinkElement to allow Link header implementation
https://bugs.webkit.org/show_bug.cgi?id=51941

Attachment 78253: Patch
https://bugs.webkit.org/attachment.cgi?id=78253&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+    RefPtr<CSSStyleSheet> m_sheet;
+    bool m_loading;
+    bool m_disabled;
+    KURL m_url;
+    RelAttribute m_relAttribute;
+    String m_type;
+    String m_media;
+    String m_charset;
+    String m_title;

It seems extremely counter-intuitive to have these in a loader class. 

+LinkLoader::~LinkLoader()
+{
+    if (sheet())
+	 sheet()->clearOwnerNode();

Link loader is not a node, why does is its destructor call clearOwnerNode()?

Maybe it's only the "LinkLoader" class name, and the refactoring is good
otherwise, but the new mix of loading, DOM and CSSOM code feels like a wrong
direction to take to me.


More information about the webkit-reviews mailing list