[webkit-reviews] review denied: [Bug 28633] Submitting a form with target=_blank works only once : [Attachment 107389] Patch

[bugzilla-daemon at webkit.org]

Andy Estes <aestes at apple.com> has denied Jon Lee <jonlee at apple.com>'s request
for review:
Bug 28633: Submitting a form with target=_blank works only once
https://bugs.webkit.org/show_bug.cgi?id=28633

Attachment 107389: Patch
https://bugs.webkit.org/attachment.cgi?id=107389&action=review

------- Additional Comments from Andy Estes <aestes at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=107389&action=review


I still think we can do better with the test. Writing tests that rely on
arbitrary timeouts is sub-optimal since it requires either using an
unnecessarily high time interval or risking flakiness. Can you instead write
this in a way that doesn't require timeouts for the test to fail? For instance,
you could queue the submissions on 0-delay timers, then add script to
submit-to-blank-multiple-times-form-action.html that logs a message and calls
notifyDone() if it's the last page to load (you could use the query string to
indicate which page load should trigger notifyDone()).

> LayoutTests/fast/forms/submit-to-blank-multiple-times.html:68
> +	   function pressEnter() {

Technically you're pressing the space bar, not the enter key. I'd rename this
to reflect the correct key press.

> LayoutTests/fast/forms/submit-to-blank-multiple-times.html:70
> +	       log('Reset global flag to false, pressing enter in second
form');

Ditto.