Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reapply the fix for #244 #952

Closed
stijnherreman opened this issue Dec 3, 2013 · 4 comments
Closed

Reapply the fix for #244 #952

stijnherreman opened this issue Dec 3, 2013 · 4 comments
Assignees

Comments

@stijnherreman
Copy link

I've prepared a test to fix #244 while not causing #521.

<form id="issue-521">
    <input type="text" name="email">
</form>
test("issue 521", function ()
{
    $("#issue-521").validate();
    $("#issue-521 input[name=email]").rules("add", "email");

    $("#issue-521 input[name=email]").val("e").keyup();
    equal($("#issue-521 label.error[for=email]:visible").length, 0, "Validation doesn't fire immediately after typing first character of email address.");

    $("#issue-521 input[name=email]").blur();
    equal($("#issue-521 label.error[for=email]:visible").length, 1, "Validation fires and reports invalid input after field loses focus.");

    $("#issue-521 input[name=email]").val("ex").keyup();
    equal($("#issue-521 label.error[for=email]:visible").length, 1, "Validation still reports invalid input after typing second character of email address.");

    $("#issue-521 input[name=email]").val("example@example.com").keyup();
    equal($("#issue-521 label.error[for=email]:visible").length, 0, "Validation fires and reports valid input immediately after typing last character of valid email address.");

    $("#issue-521 input[name=email]").val("example@example.").keyup();
    equal($("#issue-521 label.error[for=email]:visible").length, 1, "Validation fires and reports invalid input immediately after erasing last character of valid email address.");
});

With ec9c676 (2012-09-07, label 1.10.0) I can't run the test because it doesn't seem to pick up the .keyup(), but disabling the test runner, executing the first two lines of the test and manually typing a character triggers the validation, so it would cause the test to fail.

Repeating the steps with 5934f4d (2012-10-03) only triggers validation after leaving the field, so it would cause the test to succeed.

The test succeeds on the current head, commit 3f84e23. After reapplying the change from c41f2f6 (replacing this.lastElement with this.lastActive), the test still succeeds (and so do all other tests).

So next is a test for #244, simply doing the steps described in #524

<form id="issue-244">
    <input type="text" name="name">
    <input type="text" name="email" value="example@example.com">
</form>
test("issue 244", function ()
{
    $("#issue-244").validate();
    $("#issue-244 input[name=name]").rules("add", "required");
    $("#issue-244 input[name=email]").rules("add", "email");

    $("#issue-244 input[name=name]").val("abc").keyup();
    $("#issue-244 input[name=email]").focus();
    $("#issue-244 input[name=name]").focus();
    $("#issue-244 input[name=name]").val("").keyup();
    equal($("#issue-244 label.error[for=name]:visible").length, 1, "Validation fires and reports invalid input after erasing input.");
});

As expected, the test fails on the current head. After reapplying the change, the test succeeds.

Can anyone else confirm that this change can now be safely applied and that it completely fixes the issue?

@jzaefferer
Copy link
Collaborator

@nschonni could you review this?

@ghost ghost assigned nschonni Jan 15, 2014
@stijnherreman
Copy link
Author

@nschonni @jzaefferer Is there anything blocking this? The fix is available, the tests are available and the explanation/documentation is available.

@nschonni nschonni added the Bug label Mar 26, 2014
@jzaefferer
Copy link
Collaborator

@stijnherreman sorry for the delay. Reading your description again, this sounds good. Would you put the whole thing into a pull request? That would help a lot getting this landed. Thanks.

@jzaefferer
Copy link
Collaborator

I'm sorry for the lack of activity on this issue. Instead of leaving it open any longer, I decided to close old issues without trying to address them, to longer give the false impression that it will get addressed eventually, especially after several years with no activity. It doesn't mean I'm abandoning the project, just that I'm unable to work through 200+ open issues with the little time I can afford to spend on this project.

To the reporter (or anyone else interested in this issue): If you're still affected by the same issue, please consider opening a new issue, with a testpage that demonstrates the issue with a current version of the plugin. Even better, make an attempt to fix the issue yourself, and improve the project by sending a pull request. This may seem daunting at first, but you'll likely learn some useful skills that you can apply elsewhere as well. And you can help keep this project alive. We've documented how to do these things, too. A patch is worth a thousand issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants