Bug 30423 – Some HTTP tests submit a GET form to about:blank

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Yuta Kitamura]

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

[Yuta Kitamura]

Created attachment 41260 [details]
Fix some http tests to avoid submitting a form to about:blank.

[Darin Adler]

(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.

[Yuta Kitamura]

(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!

[Yuta Kitamura]

Created attachment 42471 [details]
Add a test to check the behavior of submitting a GET form to <about:blank>.

[Yuta Kitamura]

Created attachment 42472 [details]
Fix http tests so that they do not submit a GET form to about:blank.

[Eric Seidel (no email)]

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

[Yuta Kitamura]

(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.

[Yuta Kitamura]

Created attachment 42536 [details]
Fix http tests so that they do not submit a GET form to about:blank. (Use relative paths)

[Eric Seidel (no email)]

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.

[Yuta Kitamura]

Thank you for your review!

[WebKit Commit Bot]

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

[WebKit Commit Bot]

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.

[Yuta Kitamura]

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.

[Eric Seidel (no email)]

(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.

[Shinichiro Hamaji]

Committed r51069: <http://trac.webkit.org/changeset/51069>

[Shinichiro Hamaji]

Reopening to commit another patch.

[Shinichiro Hamaji]

Committed r51070: <http://trac.webkit.org/changeset/51070>