Summary: | Some HTTP tests submit a GET form to about:blank | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yuta Kitamura <yutak> | ||||||||||
Component: | Tools / Tests | Assignee: | Yuta Kitamura <yutak> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ap, darin, eric, hamaji | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Yuta Kitamura
2009-10-15 20:37:38 PDT
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> |