🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@radar
Copy link

@radar radar commented Jun 1, 2020

Summary

Possible fix for #4417.

Checklist


I am not sure where or how to test that this fix works. Help / direction would be appreciated.

@jsf-clabot
Copy link

jsf-clabot commented Jun 1, 2020

CLA assistant check
All committers have signed the CLA.

@mgol
Copy link
Member

mgol commented Jun 1, 2020

Thanks for the PR!

The most important part here is how to test this change, though, the source change itself is simple and it has already been suggested by @gibson042 in #4417.

Do you want to write a test?

@radar
Copy link
Author

radar commented Jun 1, 2020 via email

@mgol
Copy link
Member

mgol commented Jun 6, 2020

jQuery only runs unit tests in CI, we don't have any integration test setup. This poses problems in situations like this one as some focus-related bugs are specific to native events and emulating the behavior in unit tests is non-trivial.

I don't know how to write the test, I tried for a while in the past but I didn't manage to do it before I run out of time. So I can't provide a very concrete advise, unfortunately.

You would need to add a test in test/unit/event.js, you can have a look at existing focus tests there, maybe you'll find some inspiration.

@ryuran
Copy link

ryuran commented Jul 29, 2020

@radar @mgol After several months It still broken even if the fix is known. I hope it will be merged soon.
We currently are not able to update jQuery because lot of our tests fail on this valueof undefined.

@mgol
Copy link
Member

mgol commented Jul 29, 2020

@ryuran We still need to figure out a test for this. If you can help with that then the PR will surely be merged sooner.

@mgol
Copy link
Member

mgol commented Oct 9, 2020

I submitted a version with a test so I'll close this PR. Sorry! See #4799.

@mgol mgol closed this Oct 9, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants