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

[Form] deprecate submitting non HTML5 formatted dates#28724

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

Closed

Conversation

@xabbuh
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?no
BC breaks?no
Deprecations?yes
Tests pass?yes
Fixed tickets#28712 (review)
LicenseMIT
Doc PR

Technically, dates formatted according to the HTML specifications do notcontain any timezone information. But since our DateTimeType used tocontain this information in the passed, users had configure their JSlibraries to accept (and create) dates in that format.To not break BC we should accept these dates and silently ignore theadditional timezone information.
@xabbuh
Copy link
MemberAuthor

needs to be rebased once#28712 is merged up to master

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedOct 4, 2018
edited
Loading

I'm really unsure about this. Having an option means asking devs to make a choice, and making a choice is hard. Here, enabling strict mode (Yay, strict all the things!) means lowering the compatibility with the ecosystem. That's contra the robustness principle.
We have a native date parser that is excellent at parsing real-world date formats, there is no better option for me. We should leverage it.

@xabbuh
Copy link
MemberAuthor

To me, this goes along with#28753. If one wants to use HTML 5 features, they should follow the standards IMO. Custom formats are okay, but then you need to be explicit about this (i.e. sethtml5 tofalse and configure theformat option explicitly). Without being strict, there is too much room for guessing from my point of view which unnecessarily adds uncertainty that makes it hard to fix any date (and time) related bugs.

@xabbuhxabbuhforce-pushed thedatetime-strict-html5-format branch fromf545150 tod0dd6c9CompareOctober 4, 2018 14:04
// to maintain backwards compatibility we do not strictly validate the submitted date
// see https://github.com/symfony/symfony/issues/28699
if (!$this->strict) {
$regex ='/^(\d{4})-(\d{2})-(\d{2})[T ]\d{2}:\d{2}(?::\d{2})?/';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

to maintain BC, shouldn't it use the old transformer instead (which did not have that regex either, so you are only partially fixing the BC break)

@stof
Copy link
Member

stof commentedOct 4, 2018

@nicolas-grekas if you have a form type configured as HTML5_FORMAT, supporting other formats looks weird.

The option is here to opt-in for the new behavior at the time you want (needed in case your JS currently generates a different format for instance)

@stof
Copy link
Member

stof commentedOct 4, 2018

We may also want to harden DateTimeToRfc3339Transformer to actually support only RFC 3339 (usingDateTime::createFromFormat)

@xabbuhxabbuhforce-pushed thedatetime-strict-html5-format branch fromd0dd6c9 to3132819CompareOctober 4, 2018 14:23
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedOct 5, 2018
edited
Loading

I'm 👎 here: data transformers are not where strictness should apply. Strictness is for validators.
My reasoning is "why?" should we make this change? What would it improve? On the maintainer side, nothing. On the user side, it would just make using the HTTP endpoint less compatible with UI widgets into the wild, thus making things harder to use actually.
Same as why XML didn't win the race and browsers are using a loose HTML5 parser for the benefit of the robustness of the Web.

@nicolas-grekas
Copy link
Member

@xabbuh ok with my main objection: not worth adding a new option as DX may suffer?

@xabbuh
Copy link
MemberAuthor

I am closing here for now. We can consider to reopen when we notice that a less strict date format handling leads to avoidable bugs.

@xabbuhxabbuh closed thisFeb 16, 2019
@xabbuhxabbuh deleted the datetime-strict-html5-format branchFebruary 16, 2019 11:46
@nicolas-grekasnicolas-grekas modified the milestones:next,4.3Apr 30, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

4 participants

@xabbuh@nicolas-grekas@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp