[FORM] fix post_max_size_message translation #18543
Conversation
DavidBadura
commented
Apr 14, 2016
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #15479 |
| License | MIT |
| Doc PR | - |
| { | ||
| $this->serverParams = $params ?: new ServerParams(); | ||
| $this->translator = $translator; |
There was a problem hiding this comment.
You should create a translator if null is passed imo, since this can be used with Form as standalone component.
Alright forget it :)
There was a problem hiding this comment.
This is a kind of feature, shouldn't be documented in the changelog that you can pass a translator to request handlers ?
Maybe this should go on master ?
|
It's a new feature to me also, should go on master |
|
@nicolas-grekas what feature does it add ? :-) IMO translation is not feature. Error translation is a feature of Symfony. Also there is no problem merging it in 2.7, so why not ? |
|
doesn't the bug exist because of #11924? Btw. the translation already exists: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Resources/translations/validators.de.xlf#L10 but apparently it doesn't get used. i don't care whether it gets translated in 2.7 or master but it should be done at one point. |
|
So as a bug fix it should go on 2.3, right ? I agree with you and would vote to do this as a bug fix, but I doubt it gets accepted. However, if it goes in master maybe there is a better way to handle "extra" translations (not handled by the violation builder) more globally (maybe use an abstract request handler to hold some common logic), the csrf type extension also gets the translator only to pass it to its listener (even if that allows a lazy translation of the message, I wonder what consumes more memory: passing the translator in many instances or just a translated message?). |
| $this->requestHandler = $this->getRequestHandler($translator); | ||
|
|
||
| $options = array('post_max_size_message' => 'old max {{ max }}!'); | ||
| $form = $this->factory->createNamed('name', 'text', null, $options); |
There was a problem hiding this comment.
text --> Symfony\Component\Form\Extension\Core\Type\TextType
There was a problem hiding this comment.
this works only in >=2.8
|
Instead of the current implementation what do you think about adding a new form type extension that translate |
|
@aitboudad 👍 Awesome! Remains the question: 2.3 or master? |
|
To me this is a bugfix (IIRC this used to work before #11924). |
|
so, what are the next steps? is it a bug (merge in 2.3/2.7) or a feature (merge in master)? should I change it as proposed by @aitboudad? |
|
well I would consider it as a bugfix |
|
This is a bugfix. |
|
2.7 branch is the good one as this is now the oldest and still maintained branch. |
|
#19061 <- an alternative implementation as proposed by @aitboudad |
|
closing in favor of #19061 |
…id Badura) This PR was merged into the 2.7 branch. Discussion ---------- [FORM] fix post_max_size_message translation (alt. 2) | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15479, #18543 | License | MIT | Doc PR | - Commits ------- 9d8a5e5 fix post_max_size_message translation