-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Bump jquery from 3.3.1 to 3.5.1 #27266
Conversation
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.
@dependabot merge
One of your CI runs failed on this pull request, so Dependabot won't merge it. Dependabot will still automatically merge this pull request if you amend it and your tests pass. |
92ce213
to
e36b835
Compare
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.
@dependabot merge
e36b835
to
bb613d1
Compare
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.
@dependabot merge
bb613d1
to
951a2dc
Compare
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.
@dependabot merge
951a2dc
to
a291a7a
Compare
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.
@dependabot merge
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.
Needs some further careful testing as this showed some issues in #24107
This comment has been minimized.
This comment has been minimized.
a291a7a
to
8efc70b
Compare
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.
@dependabot merge
8efc70b
to
415e1b3
Compare
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.
@dependabot merge
415e1b3
to
70bda72
Compare
440cd2d
to
30d72cb
Compare
fix for the share button is here: 30d72cb let's see if more tests pass now... |
cool, only one failing JS test remains:
|
I wasn't able to find this view in the UI so couldn't test. |
regardless, I tried to debug the failing test and noticed that the "thumbnail" div is getting rendered twice, somehow it again seems like a problem with Handlebars like with the previous issues. I'm wondering if jquery is causing side effects with Handlebars or if Handlebars is using jquery internally... |
alright, I managed to fix it... once again there was an HTML element |
alright, so this is a jquery 3.5 thing: https://jquery.com/upgrade-guide/3.5/ it is said that for all which means this update is likely not safely backportable @skjnldsv FYI |
Great debugging, many thanks! That this may break apps, including custom ones, is an issue I guess, not only for backports. I'd personally still vote for merging it with NC24, also since the global jQuery is deprecated for a long time, in favour of shipping an own one for apps, but I guess that Nextcloud practice enforces a conservative approach. |
Bumps [jquery](https://github.com/jquery/jquery) from 3.3.1 to 3.6.0. - [Release notes](https://github.com/jquery/jquery/releases) - [Commits](jquery/jquery@3.3.1...3.6.0) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: npmbuildbot-nextcloud[bot] <npmbuildbot-nextcloud[bot]@users.noreply.github.com> Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
Expanded the empty span tag to resolve issue with wrong appending done by either handlebars or jquery. Signed-off-by: Vincent Petry <vincent@nextcloud.com>
22c87d8
to
ec9ea7b
Compare
restarted build due to weird failure |
Ah sorry, when rebaseing and resolving conflicts, I removed the compiled JS changes but didn't do |
/compile amend / |
Since the jquery update to 3.5.0, it seems Handlebars doesn't correctly render self-closed elements. This fixes mainfileinfodetailsview template to not use self-closed elements and fixes the JS unit tests. Signed-off-by: Vincent Petry <vincent@nextcloud.com> Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
ec9ea7b
to
49e35ff
Compare
approved by three bots but only one human, haha I agree with moving forward with this early in NC 24 so that apps can adjust when needed |
Good argument to merge it as early as possible into a new major NC version. I won't approve simply because I cannot estimate how many apps may be affected by this and whether it's better to have the need for removing the self-closing tags removed somehow announced to (app) developers earlier. So better if a second person with more insights does this. But otherwise the global jQuery has been deprecated a long time ago: https://github.com/nextcloud/server/blob/4322aab/core/src/globals.js#L105 There are so many more deprecation messages with a removal announced for NC 20 already. Probably it makes sense to be stricter with such announcements, else developers won't take them serious anymore and we're in a vicious cycle or not being able to get rid of deprecated code. Of course this means that migrating related code in core can be assured until the deadline, which is by far not the case currently 😉. |
@MichaIng I just came here to approve but our robots overlord already overruled |
Ah of course the bots merge autonomously when CI passes, sometimes I forget about that. Should be fine, and as last resort it can be easily reverted before NC24 enters RC stage. |
ok, so this only updated to 3.5.1 according to package.lock. now either we need to wait for the bot to make the proposal for 3.6.0 or do it manually? |
Probably we can wait for the weekly dependabot check tomorrow (at Saturdays). |
Bumps jquery from 3.3.1 to
3.6.03.5.1.Release notes
Sourced from jquery's releases.
Commits
0cc1ad6
3.6.0aed59da
Release: remove the need to install grunt globally8606ce4
Release: update version to 3.6.0-pre8b50fbe
Release: drop the need for npm as a local dependencya21a4b2
Release: upgrade release dependenciesc208deb
Release: update AUTHORS.txt1654874
Selector: Update Sizzle from 2.3.5 to 2.3.6f8bdb12
Support: ensure display is set to block for the support div (#4844)627c573
Build: Rename master to main across the repository15b62a2
Deferred: Rename master to primaryDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase
.Dependabot will merge this PR once CI passes on it, as requested by @nextcloud-command.
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)