[webkit-reviews] review denied: [Bug 78322] SVG should be supported in Dashboard compatibility mode : [Attachment 126443] patch

Fri Feb 10 00:02:46 PST 2012

Nikolas Zimmermann <zimmermann at kde.org> has denied  review:
Bug 78322: SVG should be supported in Dashboard compatibility mode
https://bugs.webkit.org/show_bug.cgi?id=78322

Attachment 126443: patch
https://bugs.webkit.org/attachment.cgi?id=126443&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=126443&action=review

Resetting Olivers r+, the patch doesn't apply so we can't get ews results - I'd
like to see that cr-linux ews is green, aka the expectations were updated, if
needed.
Also one test is flaky, hence setting to r-:

>
LayoutTests/http/tests/xmlhttprequest/svg-created-by-xhr-allowed-in-dashboard.h
tml:68
> +<body onLoad="loadSVG();">

s/onLoad/onload/

> LayoutTests/svg/custom/embedded-svg-allowed-in-dashboard-expected.txt:3
> + PASS: Successfully embedded SVG in document

Where does the leading space come from?

> LayoutTests/svg/custom/svg-allowed-in-dashboard-object.html:16
> +    setTimeout("timeOut()", 500);

Ouch, this is flaky, and needs to be avoided. Try listening to <body
onload="timeOut()"> instead -- you need to make sure all subresources loaded,
before calling timeOut(), though we shouldn't pray it's done in 500ms, but
instead listen to the right events.