-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
list of PRs to merge #7
Comments
@materializecss/members |
oh man, thanks for continuing the development of materialize 😄 Ehm I have this 2 years old pull request Should I rewrite this pull request for this fork? 🤔 |
Thanks, we all try our best to bring it a bit forward in our vailable free time.
I was not aware of this, good catch.
Yes, this would be great and very helpful. |
alright #9 it is 🙌 |
Dogfalo#5725 was mentioned in the community room. |
It's probably worth taking a look at Dogfalo#4391 (Dogfalo#1892) for the next feature release. |
Where are we up to regarding the select issue? That's a biggie - has it been raised/solved for the fork? (The issue is the wrong item being selected on iOS, the fix being to served an entire patched JS file after materialize.js) |
hmm I think this is issue #31. There's already a commit Dogfalo@c0da340, but it's not released officially yet. |
Correct, Dogfalo@c0da340 should be already in the current code. |
I haven't checked, but are the current CVEs resolved on the fork? They are related to XSS flaws:
|
Probably not, as fixing these would break many things as I have outlined many times (see the linked GitHub issues in the original repo). Nothing what we will do in a 1.x.x release. Also the people who reported these findings did not provide valid PoCs that could be used by attackers in valid scenarios. |
It's a bit sad that these warnings will continue to spawn on projects that are using Materialize 😕 I just saw the discussion in Dogfalo#6331 and Dogfalo#6286, this explains a lot, but is still a complex subject. Since this repository is "new", and the package will be new on NPM, maybe the vulnerability won't be registered at all, let's see 🤔 |
Still, I think input data should be sanitized by default, with a simple option in the components, and user should be able to disable it in case they want to use HTML. |
But not in a minor or patch release as this will lead to unexpected consequences like broken code and developers need to be aware of this breaking change then. We can do that in a 2.0.0 release which deprecates / removes and changes the behavior as you suggest. In v1.x we can start adding warnings to the console (and hope that developers see/read them) and mention the deprecation/change in the future. In general I would also suggest we create a documentation page about this and recommend solutions for sanitizing data with a few examples. How and what you sanitize depends on your implementation and software stack (PHP, JS, Java, ...) and where you retrieve and sanitize it. |
Regarding this statement:
Well, jQuery < 3.x has a few vulnerabilities (like XSS) but you can not patch them without breaking stuff and these versions reached their EOL. That's why I've created https://github.com/DanielRuf/snyk-js-jquery-174006 and https://github.com/DanielRuf/snyk-js-jquery-565129 which are backported patches. And these are used by many like Drupal, WordPress, TYPO3, Magento and so on. Still, the Chrome Lighthouse checks and other solutions will falsly flag them as vulnerable versions. Same for PHP and the different operating systems and companies who backport security patches. I'm aware that these things (backporting security patches) are not that great for auditors and audits. Can you open a new issue so we can discuss the correct way (documentation, planned changes, breaking changes) regarding these issues @Pierstoval? |
Will do 😄 |
Done in #38 |
It would be nice to see Dogfalo#6602 merged. It really should be possible to pass dropdownOptions to any component that uses a Dropdown. |
Have you seen PR Dogfalo#6542 / Issue Dogfalo#6273? Would be nice to see materialize keeping up with the MD guidelines |
Nope, I didn't see these. Would be great to have them as PRs in this repo. |
I've just taken a closer look - and the PR doesn't actually make any changes to the text input scss - rather it's full of random/unrelated changes to the docs afaict. It does have a bunch of CSS in the description but it'd be a pain to diff this against master to see what it's actually changing... bizarre! |
You are correct. I just took a look at the issue and not the PR to see what this was about and thought it would make sense. I didn't check the actual changes of the PR. |
Not sure if the comment with the scss code contains the actual changes and the other file changes were done by mistake (probably due to lack of experience with Git and GitHub). |
Where are things with the DropDown issue on iOS (still present in current release)? There's a solution defined in #31, but it's unclear if it's been tested. Thanks! |
Not sure. If there is still a problem with our alpha release, please open a new issue. Closing this one. Individual PRs that should be migrated should be filed as separate issues, otherwise the issues become too long and crowded. Thank you. |
I tested it on iPad and MacBook not sure about iPhone but I assume it would work there too. As @DanielRuf said, please open a new issue an provide details about Safari Version and iPhone. Thanks. |
Let's collect the open PRs that we have to merge. We already merged a few using the GitHub CLI.
For the needed steps see #2 (comment)
https://github.com/Dogfalo/materialize/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc
Already merged:
Fix some typos (found by codespell)
datepicker: add yearRangeReverse property sort year in reverse order
fix dropdown position when coverTrigger is false
Typos in upgrade guide
Planned to merge:
passive event receivers
Add input suffix
Add new i18n option 'headerFormat' to datepicker
The text was updated successfully, but these errors were encountered: