[Webkit-unassigned] [Bug 64823] <svg> fails to use explicit width and height inside <html> inside IFRAME

Wed Oct 19 01:39:23 PDT 2011

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

Nikolas Zimmermann <zimmermann at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #111542|review?                     |review+
               Flag|                            |

--- Comment #17 from Nikolas Zimmermann <zimmermann at kde.org>  2011-10-19 01:39:23 PST ---
(From update of attachment 111542)
View in context: https://bugs.webkit.org/attachment.cgi?id=111542&action=review

Great patch Levi, r=me. Please fix following issues before landing:

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:133
> +static bool isEmbeddedFrameContainingSVGDocument(const Frame* frame, RenderPart* ownerRenderer)

I'd name this "isEmbeddedThroughFrameContainingSVGDocument" - bug 47156 adds a similar method "isEmbeddedThroughSVGImage" and i'd like to have it consistent :-)

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:135
> +    // If our frame has an owner renderer, we're embedded through eg. object/embed/iframe,

I'd ASSERT(frame) here to save the reader from looking at the call-site whether this can be null.

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:136
> +    // but we only negotiate if we're in an SVG document

Trailing period missing.

> LayoutTests/svg/as-object/svg-embedded-in-html-in-iframe.html:2
> +
> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">

I'd suggest to turn this into a HTML5 test, aka. <!DOCTYPE html> and leaving out xmlns everywhere.

> LayoutTests/svg/as-object/svg-embedded-in-html-in-iframe.html:12
> +B+="<head>\r\n";
> +B+="<title>Circle</title>\r\n";
> +B+="</head><body>\r\n";

I'd remove the head alltogether, it has no benefit here. Also the \r\n are not needed.

> LayoutTests/svg/as-object/svg-embedded-in-html-in-iframe.html:14
> +B+="<ellipse cx=\"50%\" cy=\"50%\" rx=\"50%\" ry=\"50%\" fill=\"blue\" stroke=\"none\" />\r\n";

Make it a circle :-)

> LayoutTests/svg/as-object/svg-embedded-in-html-in-iframe.html:22
> +<iframe src="javascript:parent.CreateCircle();" width="100%" height="100%" frameborder="0" scrolling="no"></iframe>

Neat trick Levi! :-)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.