[webkit-reviews] review denied: [Bug 49018] [GTK] response.isNull() assert when using directory file URI : [Attachment 73407] Fix assert using directory file URI

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 9 17:03:39 PST 2010


Martin Robinson <mrobinson at webkit.org> has denied Nicolas Dufresne
<nicolas.dufresne at collabora.co.uk>'s request for review:
Bug 49018: [GTK] response.isNull() assert when using directory file URI
https://bugs.webkit.org/show_bug.cgi?id=49018

Attachment 73407: Fix assert using directory file URI
https://bugs.webkit.org/attachment.cgi?id=73407&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=73407&action=review

> LayoutTests/fast/loader/local-iFrame-directory-from-local.html:8
> +	       var localDirectoryLocation =
"file:///tmp/LayoutTests/fast/loader/resources/directory";

Sorry. I did not get a chance to give you this feedback before you posted a new
patch. Using the "/tmp" directory will not work on Windows machines. If
possible this should be changed to a directory relative to the test file.

Additionally the test should use "iframe" instead of "iFrame" to match the rest
of the tests in the directory.

> LayoutTests/fast/loader/local-iFrame-directory-from-local.html:16
> +	       var localDirectoryElement = document.createElement("iframe");
> +	       localDirectoryElement.setAttribute("id", "myDirectory");
> +	       localDirectoryElement.setAttribute("src",
localDirectoryLocation);
> +	       localDirectoryElement.setAttribute("width", "96%");
> +	       localDirectoryElement.setAttribute("height", "70%");

Is it possible to simply include this element in the HTML source instead of
adding it dynamically? You might need to use the onload event to run the test
at the proper time.

> LayoutTests/fast/loader/local-iFrame-directory-from-local.html:22
> +	       var directoryDocument = 
document.getElementById("myDirectory").contentDocument;

Is there a guarantee that the load will complete by this point?


More information about the webkit-reviews mailing list