-
Notifications
You must be signed in to change notification settings - Fork 663
Build: updating dependencies #9820
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
b784f56 to
de6d009
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.
We would need a list of plugin with their change impact (Minor/Patch)
Let's chat
de6d009 to
6e42f33
Compare
6e42f33 to
1ffb2f8
Compare
|
Regarding this:
@Garneauma Is that a wise course of action? Are modern versions of Firefox still impacted by that keyboard trap issue? IIRC Firefox used to treat Btw an upstream PR for WET's keyboard trap fix was sent back in 2017 (via dimsemenov/Magnific-Popup#945), but is still open to this day... The upstream project still seems to be active (v1.2.0 was released on June 8th), but it doesn't look like its maintainers have merged any PRs since late 2021. PS: I'm not aware of any popular web frameworks that don't officially support Firefox/Gecko. Plus it's unclear why Canada.ca's browser usage statistics would have a direct impact on WET's supported browsers (although I could see it making sense for GCWeb). If WET/GCWeb/GCDS place all of their eggs in one basket (i.e. currently popular browser engines - like WebKit/Blink) and support for other rendering engines eventually degrades to the point of being unusable... that risks "locking-in" Canada.ca to specific browsers in the long-term. That goes against the idea of interoperability and is the same kind of "IE-only" mindset many crappy sites had ages ago :P. For example, if Firefox were to regain its overall popularity in the future, that might not be reflected in Canada.ca's browser usage statistics. If users felt "forced" to switch over to another browser when visiting Canada.ca (because FF is totally broken), they'd probably get into the habit of using another browser for that specific site. End result would be statistics skewed in favour of the user's "fallback" browser. |
|
@EricDunsworth are you able to reproduce the issue with a recent version of FF version? @Garneauma can you spin up a working example to allow @EricDunsworth to test. @EricDunsworth We don't support FF anymore, if there any compatibility issue, you can submit that a PR to fix it. That can be also to update our own fork of Magnific Popup, please open a PR that update it and apply the fix and we will review it. |
|
As discussed during the technical review, let's move forward with this. @EricDunsworth will try to find the original PR for the patch of Magnific popup when he has the time in order to figure out if the patch is still needed. |
|
FWIW the original WET PR for the magnific popup plugin's keyboard trap fix was #7850. The only notable change in the There's some backstory about the change in the WET and upstream PRs. In a nutshell, it sounds like when the lightbox plugin contains an #7850 (comment) linked out to a Lightbox test page in the WET PR, but is long dead. It might be possible to replicate the affected scenario by creating a practical example using the Lightbox plugin's PS: |
Note that I am still using FF as my primary browser to perform code review and test. So it is kindly supported, but not officially because of the web stat. For sure, if there is an issue or a difference in the behaviour, we might consider to apply a fix for it. |
duboisp
left a comment
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.
Review and tested locally,
Looks good, prior to merge please wait for the same confirmation for the equivalent PR but in the GCWeb repository.
I tested more specifically the following impacted plugin
- Geomap
- Table enhancement (data table)
- Lightbox and image galary
- Multimedia player with the youtube video in a popup (do use iframe)
- Data ajax (To ensure the DOM purify update have no impact)
I do confirm that I can build and run the site/framework locally.
I ran the unit testing and all good too. 238 test has passed.
Supersedes: #9812, #9811, #9798, and #9626
Updates description
bootstrap-sass: not updatedcode-prettify: not updateddatatables.net: updated to version 1.13.11dompurify: updated to version 3.1.7es5-shim: removedfast-json-patch: not updatedhtml5shiv: removedjquery: not updatedjquery-validation: updated to version 1.21.0jsonpointer.js: not updatedmagnific-popup: updated to version 1.2.0mathjax: updated to version 3.2.2proj4: updated to version 2.14.0unorm: not updatedUpdates to the API
Datatables
Change type: patch
Files to update:
Dompurify
Change type: patch
es5-shim
Change type: patch
html5shiv
Change type: patch
jQuery Validation
Change type: patch
Files to update:
Magnific popup
Change type: patch
Mathjax
Change type: patch
Proj4
Change type: patch