Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Core: Use of jQuery attr instead of hasAttribute. (#2141)#2142

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Arkni merged 1 commit intojquery-validation:masterfromlopezcjf:master
Jul 30, 2018

Conversation

lopezcjf
Copy link
Contributor

Fixes not supported method hasAttribute on IE with enabled compatibility mode

Fixes#2141

@staabm
Copy link
Member

thx for the PR!

how to reproduce the issue you are fixing? which IE version is affected?

@staabmstaabm requested a review fromArkniFebruary 21, 2018 08:57
@lopezcjf
Copy link
ContributorAuthor

Hi,
I reproduced this error by setting up a basic form with 2 required fields and using IE 11 with enabled compatibility mode (Go to settings -> Compatibility View Settings -> Add the website address, in my scenario is localhost)

Open the page
Click on the first field
Then click on the second field

When focusing out from the first field, the error appears.

@lopezcjf
Copy link
ContributorAuthor

This is my validation setup:

$('.login-form').validate({    errorElement: 'span',     errorClass: 'help-block',     focusInvalid: false,     rules: {        UserName: {            required: true        },        Password: {            required: true        }    },    messages: {        UserName: {            required: resources.IngresarUsuario        },        Password: {            required: resources.IngresarPWD        }    },    invalidHandler: function (event, validator) {         $('.alert-danger', $('.login-form')).show();    },    highlight: function(element) {         $(element)            .closest('.form-group').addClass('has-error');     },    success: function (label) {        $(label).closest('.form-group').removeClass('has-error');        $(label).remove();    },    errorPlacement: function (error, element) {        $(error).insertAfter($(element).closest('.input-icon'));    }});

@Arkni
Copy link
Member

@staabm

how to reproduce the issue you are fixing? which IE version is affected?

hasAttribute is supported as of IE8. And because we still support down to jQuery 1.7.2 which in its own support IE 7, we have to replace all the 4 occurences ofhasAttribute with their equivalent in jQuery using.attr().
That's the main reason why I added thebug label to the linked issue.

We definetly should drop older versions of jQuery in the next major release and stick to versions>v1.10.0,>2.2.0 andv3.0.0 or just support the latest stable version (v3.x.y)

staabm reacted with thumbs up emoji

@staabm
Copy link
Member

staabm commentedFeb 22, 2018
edited
Loading

That's the main reason why I added the bug label to the linked issue.

hmm either the linked issue was added after I did the comment or I missed it completely.
thx for the hint.

We definetly should drop older versions of jQuery in the next major release and stick to versions >v1.10.0, >2.2.0 and v3.0.0 or just support the latest stable version (v3.x.y)

big 👍 .

we should gather some information which usefull BC breaks we should target for 2.0.

@Arkni
Copy link
Member

Arkni commentedMar 3, 2018
edited
Loading

Hi@lopezcjf,

Sorry for taking so long to review this.

I did some research in order to have a global understanding of this issue and I was wrong suggesting using.attr() to fix this.
I found out thatcontenteditable is supported as ofIE6 and so we can easily replace the check withelement.isContentEditable which returnstrue is the element is editable otherwise it returnsfalse.
This will help us with the case where the element has nocontenteditable attribute but it inherits the editable state from its editable parent (contenteditable = "inherit").

So, can you please update this PR to useisContentEditable property instead.

@Arkni
Copy link
Member

Arkni commentedJun 16, 2018
edited
Loading

Hi@lopezcjf,

Just a friendly reminder.

Are you still willing to continue this PR? No worries if you don't have time for it, though.

Thanks!

Copy link
Member

@ArkniArkni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Hi@lopezcjf,

I did the changes I requested in my comment#2142 (comment) on your behalf. Don't worry, I kept the attribution intact (the commit is still authored by you).

I'll merge this PR now.

Thansk for your contribution :)

@ArkniArkni merged commit7c22d68 intojquery-validation:masterJul 30, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ArkniArkniArkni approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@lopezcjf@staabm@Arkni

[8]ページ先頭

©2009-2025 Movatter.jp