[webkit-reviews] review granted: [Bug 44567] If an EMBED is part of an OBJECT's fallback content, WebKit should only render the EMBED if the OBJECT fails to load. : [Attachment 65474] Patch (v2)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 25 14:28:51 PDT 2010
Darin Adler <darin at apple.com> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 44567: If an EMBED is part of an OBJECT's fallback content, WebKit should
only render the EMBED if the OBJECT fails to load.
https://bugs.webkit.org/show_bug.cgi?id=44567
Attachment 65474: Patch (v2)
https://bugs.webkit.org/attachment.cgi?id=65474&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
> + If an EMBED is part of an OBJECT's fallback content, WebKit should
only
> + render the EMBED if the OBJECT fails to load.
Those capitalized tag names are so old school! I think <embed> and <object> are
so much more 2010.
> Node* p = parentNode();
> if (p && p->hasTagName(objectTag)) {
> ASSERT(p->renderer());
> - return false;
> + if (!static_cast<HTMLObjectElement*>(p)->useFallbackContent())
> + return false;
> }
A "why" comment would be so great here.
> + bool useFallbackContent() const { return m_useFallbackContent; }
My only concern about exposing this is that I don't know when the value is set
up. For example, might HTMLEmbedElement::rendererIsNeeded call this function
before the <object> has decided whether to use fallback content?
When is it safe to call this? Maybe only when we have made a renderer? Can we
add some kind of assertion?
> - url = p->value();
> + url = deprecatedParseURL(p->value());
This looks like a good bug fix, but perhaps one that is separate from the main
change here. Did you add a test case to cover this fix? Should we land this fix
separately, first?
> + // Turn the attributes of the OBJECT tag into arrays, but don't
override PARAM values.
I think "OBJECT tag" and "PARAM" should be either "object element" or "<object>
element" and "<param> values".
> + * accessibility/plugin.html: Simplified this test by removing the
> + unnecessary <object> around the <embed>.
But don't we also want test coverage for the common case where an <object> is
around an <embed>?
More information about the webkit-reviews
mailing list