Bug 82572

Created attachment 134495 [details] Patch

Comment on attachment 134495 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134495&action=review > Source/WebCore/dom/ShadowRoot.cpp:-116 > - ASSERT(purpose != CreatingUserAgentShadowRoot || !element->hasShadowRoot()); Looks like this assertion should be corrected instead of removed. ASSERT(purpose == CreatingUserAgentShadowRoot || !element->hasShadowRoot()) will be an expected form. > Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:80 > + ASSERT(existingRoot->firstChild() == existingRoot->lastChild()); You mean existingRoot->childNodeCount() == 1 ? > Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:82 > + static_cast<HTMLElement*>(existingRoot->firstChild())->setInlineStyleProperty(CSSPropertyWebkitBoxFlex, 1.0, CSSPrimitiveValue::CSS_NUMBER); toHTMLElement() has captured this assert-and-cast pattern.

Comment on attachment 134495 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134495&action=review >> Source/WebCore/dom/ShadowRoot.cpp:-116 >> - ASSERT(purpose != CreatingUserAgentShadowRoot || !element->hasShadowRoot()); > > Looks like this assertion should be corrected instead of removed. > ASSERT(purpose == CreatingUserAgentShadowRoot || !element->hasShadowRoot()) will be an expected form. Really? Your expected form means that one can't add multiple author shadow roots to an element. >> Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:80 >> + ASSERT(existingRoot->firstChild() == existingRoot->lastChild()); > > You mean existingRoot->childNodeCount() == 1 ? Yes. I thought I should have avoided childNodeCount because it's O(N), but it's acceptable because the child node count should be 1. I'll change it to childNodeCount()==1. >> Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:82 >> + static_cast<HTMLElement*>(existingRoot->firstChild())->setInlineStyleProperty(CSSPropertyWebkitBoxFlex, 1.0, CSSPrimitiveValue::CSS_NUMBER); > > toHTMLElement() has captured this assert-and-cast pattern. will do.

Created attachment 134499 [details] Patch 2

(In reply to comment #3) > (From update of attachment 134495 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134495&action=review > > >> Source/WebCore/dom/ShadowRoot.cpp:-116 > >> - ASSERT(purpose != CreatingUserAgentShadowRoot || !element->hasShadowRoot()); > > > > Looks like this assertion should be corrected instead of removed. > > ASSERT(purpose == CreatingUserAgentShadowRoot || !element->hasShadowRoot()) will be an expected form. > > Really? >> Your expected form means that one can't add multiple author shadow roots to an element. You're right. I was confused. > > >> Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:80 > >> + ASSERT(existingRoot->firstChild() == existingRoot->lastChild()); > > > > You mean existingRoot->childNodeCount() == 1 ? > > Yes. I thought I should have avoided childNodeCount because it's O(N), but it's acceptable because the child node count should be 1. I'll change it to childNodeCount()==1. > > >> Source/WebCore/html/shadow/TextFieldDecorationElement.cpp:82 > >> + static_cast<HTMLElement*>(existingRoot->firstChild())->setInlineStyleProperty(CSSPropertyWebkitBoxFlex, 1.0, CSSPrimitiveValue::CSS_NUMBER); > > > > toHTMLElement() has captured this assert-and-cast pattern. > > will do.

Comment on attachment 134499 [details] Patch 2 Clearing flags on attachment: 134499 Committed r112503: <http://trac.webkit.org/changeset/112503>

All reviewed patches have been landed. Closing bug.