RESOLVED FIXED78322
SVG should be supported in Dashboard compatibility mode 
https://bugs.webkit.org/show_bug.cgi?id=78322
Summary SVG should be supported in Dashboard compatibility mode
Tim Horton
Reported 2012-02-09 20:22:09 PST
We should revert http://trac.webkit.org/changeset/21418 <rdar://problem/5861278> I have a patch.
Attachments
patch (34.76 KB, patch)
2012-02-09 20:33 PST, Tim Horton 		zimmermann: review-
DetailsFormatted DiffDiff
patch (38.49 KB, patch)
2012-02-10 12:22 PST, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (36.09 KB, patch)
2012-02-20 16:18 PST, Tim Horton 		dino: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Tim Horton
Comment 1 2012-02-09 20:33:46 PST
Created attachment 126443 [details] patch
Nikolas Zimmermann
Comment 2 2012-02-10 00:02:47 PST
Comment on attachment 126443 [details] patch 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.html: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.
Tim Horton
Comment 3 2012-02-10 00:04:27 PST
(In reply to comment #2) > (From update of attachment 126443 [details]) > 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. The patch doesn't apply, but not for any discernible reason. > Also one test is flaky, hence setting to r-: > > > LayoutTests/http/tests/xmlhttprequest/svg-created-by-xhr-allowed-in-dashboard.html: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. Hmm! I didn't look at the tests, I just swapped all the "PASS" and "FAIL"s :-D
Tim Horton
Comment 4 2012-02-10 12:22:02 PST
Created attachment 126553 [details] patch Fixed Niko's concerns; I'm sure the patch still won't apply (I bet it's the git mv that is broken), we can make any changes for other platforms later, but I don't expect there will be any.
Dean Jackson
Comment 5 2012-02-20 15:21:28 PST
Comment on attachment 126553 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=126553&action=review Looks ok to me, but I was concentrating on code changes not tests. Maybe Niko wants another look? It would be nice to resubmit and verify that the bots compile, etc. > LayoutTests/http/tests/xmlhttprequest/svg-created-by-xhr-allowed-in-dashboard.html:6 > +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> > +<html> > +<head> > +<title>Test to ensure SVG is enabled in Dashboard compatibility mode</title> > +<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> > +<script language="JavaScript" type="text/JavaScript"> Just do: <!doctype html> <html> <head> <title>... <script> <- no attributes I realise you're just moving (and slightly editing) the file, but might as well clean it up.
Dean Jackson
Comment 6 2012-02-20 15:23:14 PST
Comment on attachment 126553 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=126553&action=review > LayoutTests/svg/custom/svg-allowed-in-dashboard-object.html:2 > +<body onload="timeOut()"> See below. > LayoutTests/svg/custom/svg-allowed-in-dashboard-object.html:16 > + function timeOut() { This function was named timeOut because it was being called from a setTimeout. I think we should rename it runTest or something.
Tim Horton
Comment 7 2012-02-20 16:18:29 PST
Created attachment 127866 [details] Patch
Tim Horton
Comment 8 2012-02-20 16:19:23 PST
Just tried webkit-patch for the first time, let's see if that makes a patch the bots can deal with. Other than that, I think I've addressed your changes.
Tim Horton
Comment 9 2012-02-24 11:41:47 PST
Landed in http://trac.webkit.org/changeset/108832
Note You need to log in before you can comment on or make changes to this bug.