Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Additionals: Added maxsize plugin#1512

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

twolfson
Copy link

We recently created a maxsize plugin to detect that a file is too large before submission. Based off of#450 and#1111, it looks like other people would like it as well. In this PR:

  • Added maxsize plugin intosrc/additional

Missing:

We didn't add any tests as there are no tests for the only otherFileList based plugin (accept.js). I am assuming that the reason for this is we cannot programmatically tell a browser where to get our file uploads from.

@twolfsontwolfsonforce-pushed thedev/add.max.size.sqwished branch fromeaca996 toe2ea371CompareJuly 8, 2015 22:28
@jzaefferer
Copy link
Collaborator

Thank you for the contribution. This looks like a good start, but very much needs tests. There is an open PR that added tests for the accept method (#1373) and one more that further extends the method (#1441). Both need some rebasing or other fixes to get merged. Could I interest you in that? If so, start with the accept PRs (rebase and whatever else is needed), then rebase this PR on top and share the testing infrastructure (theacceptFileDummyInput function).

@twolfson
Copy link
Author

imo, per-computer filetype validation isn't too trustworthy as it can change a lot from computer to computer (e.g. we have seen.pdf with the type ofapplication/download,www/unknown,application/x-pdf). As a result, I would prefer to stay away from#1441 and its edge cases.

Is there anything that needs to be done in#1373 aside from a squash/rebase?

@jzaefferer
Copy link
Collaborator

No, I think that's all there is to it.

@twolfson
Copy link
Author

Is there any reason why I should do it over you? I am not quite sure I follow the problem =/ The attribution should still be preserved -- here is an example injscs:jscs-dev/node-jscs@18b4ffc

@twolfson
Copy link
Author

If you are unfamiliar with how to grab commits from a PR outside of GitHub, here is a gist describing the details:

https://gist.github.com/piscisaureus/3342247

@jzaefferer
Copy link
Collaborator

The problem are the 7 commits on that PR, of which at least three produce conflicts during a rebase. I figured you could help resolve those, since you'd likely end up integrating some of those changes into your own PR anyway.

@twolfson
Copy link
Author

Taking a shot at getting#1373 back on its feet

staabm pushed a commit that referenced this pull requestNov 26, 2015
This adds support for types like "application/epub+zip"which contain regex meta characters.Fixes#1243,#1258.Closes#1531,#1373. Refs#1512.
@ghost
Copy link

ghost commentedSep 22, 2016
edited by ghost
Loading

@jzaefferer@twolfson Please look at#6834.

@twolfson
Copy link
Author

This PR was opened over a year ago, I no longer work at the organization I started it from, and I no longer have time to contribute to this PR. Closing this PR -- someone can pull over the changes into another PR if they want them

Invis1ble reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@twolfson@jzaefferer@staabm

[8]ページ先頭

©2009-2025 Movatter.jp