[webkit-reviews] review granted: [Bug 48980] Implement form validation message UI : [Attachment 78886] Patch 4
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jan 13 21:21:39 PST 2011
Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Kent Tamura
<tkent at chromium.org>'s request for review:
Bug 48980: Implement form validation message UI
https://bugs.webkit.org/show_bug.cgi?id=48980
Attachment 78886: Patch 4
https://bugs.webkit.org/attachment.cgi?id=78886&action=review
------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78886&action=review
Let's give it a spin. I am not sure yet whether the design with the holder
class (ValidationMessage) is what we should use, but I don't know for sure. And
please appease the style elf. He's angry.
> Source/WebCore/css/CSSStyleSelector.cpp:984
> + (s->shadowPseudoId() == m_element->shadowPseudoId()) &&
Yay! Awesome.
> Source/WebCore/dom/Node.cpp:1384
> + if (parentRenderer && (parentRenderer->canHaveChildren() ||
isShadowRoot()) && parent->childShouldCreateRenderer(this)) {
This is wrong, because we do need to respect canHaveChildren() on the renderer.
Or do we? This is an interesting question. Can you at least put a FIXME at the
top and file a bug to investigate this. So far, this is still a hack.
> Source/WebCore/html/ValidationMessage.cpp:120
> + // FIXME: We need a way to host multiple shadow roots in a single node.
In XBL2, this is solved with inheritance, not multiple shadow roots. But yeah,
this looks like a hack.
> Source/WebCore/html/ValidationMessage.cpp:125
> + m_bubble->attach();
Shouldn't need this anymore. All attachment/detachment is done through the
standard means. Perhaps we could just setNeedsStyleRecalc or something like
that? At least put a FIXME here to investigate.
More information about the webkit-reviews
mailing list