-
Notifications
You must be signed in to change notification settings - Fork 476
Core: Warn on use of jQuery.holdReady() #247
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
Conversation
@dmethvin, thanks for your PR! By analyzing the history of the files in this pull request, we identified @mgol, @markelog and @gibson042 to be potential reviewers. |
|
||
expectWarning( "jQuery.holdReady", 1, function() { | ||
jQuery.holdReady( true ); | ||
jQuery.holdReady( false ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you calling it twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be sure I don't get two warnings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
@Krinkle I remember you were using it somewhere? |
@markelog I used to use it at Wikipedia by calling It was mainly there to work around a bug in Firefox (as well as minor optimisation to trigger the event when its "early" enough for us, which was a bad idea in hindsight). The bug was actually caused by some fragile logic on our end, fixed and refactored in 2014, and the For third-party users of MediaWiki, this was released in 1.22.0 (Dec 2013) and removed in 1.26.0 (Nov 2015). The current stable (1.28) and LTS (1.27) no longer have this, however we do have a few months left until May 2017 before MediaWiki 1.23 (legacy LTS) reaches end-of-life. |
@Krinkle thank you for the thoughtful comment, looks like we are in the clear :) |
Fixes #225
jQuery core includes two implementations of
jQuery.holdReady()
and it would be tough at this point to anticipate how we might rewrite it, so I skipped trying to fill it for now at least.