[Webkit-unassigned] [Bug 20212] [XBL] Add full support for element="" attachment

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=20212


eric at webkit.org changed:

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




------- Comment #3 from eric at webkit.org  2008-08-01 15:26 PDT -------
(From update of attachment 22552)
Shouldn't:
 49         String currentAttr = getAttribute(XBLNames::elementAttr);

Just be:
attr->value(); ?

+        Document* boundDocument = document()->isXBLDocument() ? 0 :
document();
is confusing to me.  What would we do in the XBLDocument case?

Um... we'll crash here:
+        // Update the styleSelector so that bindings are updated.
+        // FIXME: is it always required?
+        boundDocument->updateStyleSelector();

This patch crashes for inline XBL.  Can't land it that way.

Argument names are not neede here:
+        XBLBindingElement(const WebCore::QualifiedName& qName,
WebCore::Document* doc); 

The fist one is not needed here:
+        virtual void attributeChanged(Attribute* attr, bool preserveDecls);

This should use early return, or early ASSERT:
+void XBLBindingManager::removeBindingSheet(Document* document,
PassRefPtr<CSSStyleSheet> sheet)
+{
+    StyleSheetVector* documentSheets = m_bindingSheets.get(document);
+
+    if (documentSheets) {
+        for (StyleSheetVector::iterator it = documentSheets->begin(); it !=
documentSheets->end(); ++it)
+            if (*it == sheet) {
+                documentSheets->remove(it - documentSheets->begin());
+                return;
+            }
+    }
+    // We should have the sheet in our binding sheets.
+    ASSERT_NOT_REACHED();

is it OK to call removeBindingSheet after the document has already been removed
from the sheets list?  Part of the code seems to assume it is, part doesn't...

You need to include a test case which tests the inline xbl case.


-- 
Configure bugmail: https://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.