test(*): fix tests involving submit on Chrome 60
#16141
Conversation
| @@ -420,15 +423,18 @@ describe('form', function() { | |||
| inject(function($timeout) { | |||
| doc = jqLite('<div>' + | |||
| '<form ng-submit="submitMe()">' + | |||
| '<button ng-click="destroy()"></button>' + | |||
| '<button type="submit" ng-click="destroy()"></button>' + | |||
There was a problem hiding this comment.
Is this necessary? I think if there's no other button, then the first button is the submit button
There was a problem hiding this comment.
No, it is not necessary. Just making it more explicit for people that might not know.
| @@ -386,6 +386,9 @@ describe('form', function() { | |||
| doc = jqLite('<form ng-submit="submitMe()">' + | |||
| '<input type="submit" value="submit">' + | |||
| '</form>'); | |||
| // Support: Chrome 60+ (on Windows) | |||
| // We need to add the form to the DOM in order for `submit` events to be properly fired. | |||
| window.document.body.appendChild(doc[0]); | |||
There was a problem hiding this comment.
Aren't all of these memory leaks? Or is there some global cleanup between tests?
There was a problem hiding this comment.
We empty body before each test.
test/ng/directive/formSpec.js
Outdated
| @@ -460,6 +466,7 @@ describe('form', function() { | |||
| .runs(function() { | |||
| expect(doc.html()).toBe(''); | |||
| expect(destroyed).toBe(true); | |||
| expect(reloadPrevented).toBe('never called'); | |||
There was a problem hiding this comment.
Is this unrelated to the other changes?
There was a problem hiding this comment.
The test results disagree with this expectation
👀
⭕️
And based on the description I also would expect that this is true.
There was a problem hiding this comment.
They agree locally (probably a Windows thing) 😕
There was a problem hiding this comment.
So, it turns out the cause is again that Chrome 60+ on Windows does not fire submit events if the form is not attached to the DOM. Changed the expectation to verify that the submit event is eithr never fired or (if fired) the reload is prevented.
|
Looks like chrome 60 is not yet available |
gkalpak
force-pushed
the
test-fix-submit-on-Windows
branch
from
dca8e13 to
0e2f4d8
Compare
|
Yeah. Trying 59 😃 |
On Chrome 60 (at least on Windows) the `submit` event when clicking on a submit button is not fired on the form element, unless it is already part of the DOM.
gkalpak
force-pushed
the
test-fix-submit-on-Windows
branch
2 times, most recently
from
611631a to
527fe6f
Compare
gkalpak
force-pushed
the
test-fix-submit-on-Windows
branch
from
527fe6f to
4a3c001
Compare
|
Also updated to latest version for Chrome (59) and Firefox (54) available on Saucelabs. |
…ble on Saucelabs Closes #16141
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Test fix.
What is the current behavior? (You can also link to an open issue here)
Some tests involving
submitfail on Windows with Chrome 60.What is the new behavior (if this is a feature change)?
All tests pass.
Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Docs have been added / updated (for bug fixes / features)Other information:
On Chrome 60 (at least on Windows) the
submitevent when clicking on a submit button is not fired on the form element, unless it is already part of the DOM.