The following tests depend on an assumption that submitting an empty form to <about:blank> navigates to <about:blank?>. LayoutTests/http/tests/navigation/onload-navigation-iframe.html LayoutTests/http/tests/navigation/onload-navigation-iframe-2.html LayoutTests/http/tests/navigation/onload-navigation-iframe-timeout.html However, this behavior is not common among other browsers (neither Firefox nor IE do). Actually, Chromium's layout test runner is failing to run these tests due to this reason. Chromium Bug: http://code.google.com/p/chromium/issues/detail?id=20570
Created attachment 41260 [details] Fix some http tests to avoid submitting a form to about:blank.
(In reply to comment #0) > However, this behavior is not common among other browsers (neither Firefox nor > IE do). So you’re saying, I think, that this is a bug in WebKit that does not affect Chromium but does affect Safari. Given that, removing the tests that show the bug seems like the wrong thing to do. While it’s good to remove the accidental reliance on this, there should be a new test showing the problem and a bug report about it. It’s good for Chromium that this WebKit bug does not affect Chromium, but we should aim to have such things be correct in other ports as well.
(In reply to comment #2) > Given that, removing the tests that show the bug seems like the wrong thing to > do. While it’s good to remove the accidental reliance on this, there should be > a new test showing the problem and a bug report about it. I agree. I will create another patch that adds a separate test that checks this behavior sometime next week. Thank you for your comment!
Created attachment 42471 [details] Add a test to check the behavior of submitting a GET form to <about:blank>.
Created attachment 42472 [details] Fix http tests so that they do not submit a GET form to about:blank.
Comment on attachment 42472 [details] Fix http tests so that they do not submit a GET form to about:blank. Why do these test need the domain/port in the URL? Why can't they just be relative to /? http://127.0.0.1:8000/navigation/resources/blank.txt
(In reply to comment #6) > (From update of attachment 42472 [details]) > Why do these test need the domain/port in the URL? Why can't they just be > relative to /? > > http://127.0.0.1:8000/navigation/resources/blank.txt We can use relative path in "action" attribute in <form>. But back forward list dump (in *-expected.txt files) will not change, as it always prints full URLs. Anyway I agree that it's a bit odd. I think we can use a path relative to current directory, i.e. "resources/blank.txt", instead of full path, so I want to recreate this patch accordingly.
Created attachment 42536 [details] Fix http tests so that they do not submit a GET form to about:blank. (Use relative paths)
Comment on attachment 42536 [details] Fix http tests so that they do not submit a GET form to about:blank. (Use relative paths) Seems reasonable.
Thank you for your review!
Comment on attachment 42471 [details] Add a test to check the behavior of submitting a GET form to <about:blank>. Rejecting patch 42471 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11582 test cases. fast/js/global-constructors.html -> failed Exiting early after 1 failures. 7076 tests run. 131.39s total testing time 7075 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 2 test cases (<1%) had stderr output
Comment on attachment 42536 [details] Fix http tests so that they do not submit a GET form to about:blank. (Use relative paths) Rejecting patch 42536 from commit-queue. Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Eric Seidel', '--force']" exit_code: 2 Last 500 characters of output: iframe-2-expected.txt patching file LayoutTests/http/tests/navigation/onload-navigation-iframe-2.html patching file LayoutTests/http/tests/navigation/onload-navigation-iframe-expected.txt patching file LayoutTests/http/tests/navigation/onload-navigation-iframe-timeout-expected.txt patching file LayoutTests/http/tests/navigation/onload-navigation-iframe-timeout.html patching file LayoutTests/http/tests/navigation/onload-navigation-iframe.html patch: **** Only garbage was found in the patch input.
Both patches were rejected by commit-queue. - Patch #1 hit a problem that adding a test makes fast/js/global-constructors.html fail. Hamaji found the same problem and he is investigating the issue. After the issue is resolved, we can put it into commit-queue again. https://bugs.webkit.org/show_bug.cgi?id=31286 - Patch #2 failed because svn-apply didn't like git's patch output of adding an empty file (blank.txt). I don't know the workaround, and maybe we need to commit manually.
(In reply to comment #13) > - Patch #2 failed because svn-apply didn't like git's patch output of adding an > empty file (blank.txt). I don't know the workaround, and maybe we need to > commit manually. This is bug 29684.
Committed r51069: <http://trac.webkit.org/changeset/51069>
Reopening to commit another patch.
Committed r51070: <http://trac.webkit.org/changeset/51070>