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

Update jQuery version of default public page and other subscribe pages #511

Merged
merged 5 commits into from Apr 11, 2019

Conversation

Projects
None yet
3 participants
@xh3n1
Copy link
Member

commented Apr 9, 2019

xh3n1 added some commits Apr 9, 2019

Update jQuery version of default public pages
Signed-off-by: Xheni Myrtaj <myrtajxheni@gmail.com>

@xh3n1 xh3n1 requested a review from samtuke Apr 9, 2019

@michield

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

Can we add some behat checks to avoid this from happening?

@xh3n1

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

@suelaP will add the test in a separate PR.

@samtuke

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

@xh3n1 What about renaming the jQuery files, or using a symlink, to avoid having to update these references the next time that jQuery is updated?

@xh3n1

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

@samtuke OK, I will rename the jQuery files to jquery.min.js and update the references, does that sound good?

@samtuke

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Yes, so long as there is still a way to identify the version. Either within the file itself (if the version is stated), or if necessary, perhaps a simple version text file stating the jquery version.

@samtuke

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Can this check and replacement also be applied to all existing subscribe pages stored in the database? That would provide a more seamless solution for users - their subscribe pages would be automatically updated to the latest jquery version, instead of them having to reset or replace them.

@xh3n1 xh3n1 changed the title Update jQuery version of default public pages Update jQuery version of default public page and other subscribe pages Apr 11, 2019

$newFooter = str_replace($pattern, $replacement, $oldFooter);
SaveConfig('pagefooter', $newFooter);
}
$replacement = "min.js";

This comment has been minimized.

Copy link
@samtuke

samtuke Apr 11, 2019

Contributor

Is there any disadvantage to these patterns being more specific, like 'jquery-1.12.1.min.js' and 'jquery.min.js' to avoid unforeseeable false positives?

This comment has been minimized.

Copy link
@xh3n1

xh3n1 Apr 11, 2019

Author Member

@samtuke no, I will do that now as it sounds safer.

This comment has been minimized.

Copy link
@samtuke

samtuke Apr 11, 2019

Contributor

Thanks!

Use fullname of filename as pattern
Signed-off-by: Xheni Myrtaj <myrtajxheni@gmail.com>
@xh3n1

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

@samtuke done, thanks. The version can be found in the filename or in browser console ( jQuery.fn.jquery)

@samtuke

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

@xh3n1 That console command is useful, thanks. When you say the filename, you mean there is a duplicate copy of the library with the version in the name, or which filename?

@xh3n1

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

@samtuke the jQuery version itself is defined inside jquery.min.js. I didn't mean duplicated copy.

@samtuke samtuke merged commit b11563c into master Apr 11, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.