[TwigBridge] fixes
merged branch jfsimon/issue-6747 (PR #7281)

This PR was squashed before being merged into the 2.1 branch (closes #7281).

Commits
-------

e3547c6 [TwigBridge] fixes

Discussion
----------

[TwigBridge] fixes `TranslationDefaultDomainNodeVisitor`

... by adding scope management.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #6747

---------------------------------------------------------------------------

by vicb at 2013-03-06T15:14:57Z

Are `open` and `close` good names ? I would prefer `addChild` and `getParent` as there is nothing that is really opened or closed ?

Edit: by looking at the code a second time, I think open & close make sense .

---------------------------------------------------------------------------

by jfsimon at 2013-03-07T13:46:13Z

@vicb I know this is not consistent with the rest of the framework.
I dont think @fabpot will like them.

---------------------------------------------------------------------------

by stof at 2013-03-07T13:53:14Z

what about enter and leave ? It would be consistent with the naming used for scopes in the DI component

---------------------------------------------------------------------------

by vicb at 2013-03-07T13:59:35Z

@stof I like your proposal. My main concern being "leave" (former "close"), @jfsimon could you throw when a scope is used after it has been left ?

---------------------------------------------------------------------------

by jfsimon at 2013-03-07T14:01:56Z

go for enter/leave, but why throwing an exception on using a leaved scope?

---------------------------------------------------------------------------

by vicb at 2013-03-07T14:05:22Z

because if you have a ref to a left scope, you can still use it which is bad
[Translation] added xliff loader/dumper with resname support
merged branch jfsimon/issue-5940 (PR #7304)

This PR was squashed before being merged into the 2.1 branch (closes #7304).

Commits
-------

5bc30bb [Translation] added xliff loader/dumper with resname support

Discussion
----------

[Translation] added xliff loader/dumper with resname support

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #5940

---------------------------------------------------------------------------

by jfsimon at 2013-03-08T15:49:18Z

@Tobion what do you think of `XliffFileWithResnameDumper` and `XliffFileWithResnameLoader`?

---------------------------------------------------------------------------

by robinduval at 2013-03-08T15:49:58Z

:100: great idea !

---------------------------------------------------------------------------

by lizjulien at 2013-03-08T15:51:45Z

:star2: better.

---------------------------------------------------------------------------

by saro0h at 2013-03-08T15:52:50Z

:+1:  Can't be worse than the previous one

---------------------------------------------------------------------------

by dxb at 2013-03-08T16:00:32Z

+1

---------------------------------------------------------------------------

by Tobion at 2013-03-08T16:07:59Z

@jfsimon yes the name make sense. It is not possible to add this feature without BC break?
And if not, why not simply add an option/enable method to the existing classes. Having new classes for this seems awkward to me.

---------------------------------------------------------------------------

by jfsimon at 2013-03-08T16:17:44Z

@Tobion I totaly agree with you, but this has been discussed in #5940.
Does the presence of the `resname` attribute in the `trans-unit` tags represent a BC break?
Would opening another PR with this solution be a good idea?

---------------------------------------------------------------------------

by stof at 2013-03-08T17:01:00Z

Your XliffFileWithResnameLoader is able to load all files supported by XliffFileLoader so IMO you don't need to add a new class.

---------------------------------------------------------------------------

by jfsimon at 2013-03-08T18:21:32Z

@stof would the `resname` attribute addition in the dumped XLIFF be considered as a BC break?

---------------------------------------------------------------------------

by jfsimon at 2013-03-09T13:16:32Z

@stof done.
@mvrhov done.
[Translation] removed wriong 'use'
[BrowserKit] added ability to ignored malformed set-cookie header
[TwigBridge] fixed non probant tests & added new one
[TwigBridge] fixed fixed scope & trans_default_domain node visitor
[TwigBridge] now enter/leave scope on Twig_Node_Module
merged branch jfsimon/issue-7304 (PR #7345)

This PR was merged into the 2.1 branch.

Commits
-------

ae0b8ea [Translation] removed wriong 'use'

Discussion
----------

[Translation] removed wrong 'use'

Fixes last comment from #7304 // cc @stof
merged branch jfsimon/issue-7281 (PR #7348)

This PR was merged into the 2.1 branch.

Commits
-------

5ad4bd1 [TwigBridge] now enter/leave scope on Twig_Node_Module
fe4cc24 [TwigBridge] fixed fixed scope & trans_default_domain node visitor
c5e999a [TwigBridge] fixed non probant tests & added new one

Discussion
----------

[TwigBundle] fixes errors with the default domain node visitor & scope

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7281

---------------------------------------------------------------------------

by Julien-Webgenery at 2013-03-12T16:35:58Z

:+1:

---------------------------------------------------------------------------

by lennerd at 2013-03-12T16:38:40Z

:+1:

---------------------------------------------------------------------------

by rc2c at 2013-03-12T16:53:04Z

:+1:
merged branch jfsimon/issue-7039 (PR #7347)

This PR was merged into the 2.1 branch.

Commits
-------

fc47589 [BrowserKit] added ability to ignored malformed set-cookie header

Discussion
----------

[BrowserKit] adds ability to ignore malformed set-cookie header

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7039
[FrameworkBundle] fixed cahe:clear command's warmup
[2.1][TwigBridge] Fixes Issue #7342 in TwigBridge
merged branch benbender/2.1 (PR #7344)

This PR was squashed before being merged into the 2.1 branch (closes #7344).

Commits
-------

c423f16 [2.1][TwigBridge] Fixes Issue #7342 in TwigBridge

Discussion
----------

[2.1][TwigBridge] Fixes Issue #7342 in TwigBridge

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7342
| License       | MIT
| Doc PR        | --

---------------------------------------------------------------------------

by stof at 2013-03-12T13:28:15Z

Can you add a test to avoid regressions ?

---------------------------------------------------------------------------

by benbender at 2013-03-12T13:54:02Z

Done

---------------------------------------------------------------------------

by stevelacey at 2013-03-12T14:40:59Z

Looks good to me?

---------------------------------------------------------------------------

by pborreli at 2013-03-12T16:04:27Z

:+1:
[TwigBridge] removed double var initialization (refs #7344)
[Validator] fix showing wrong max file size for upload errors

this was because the maxSize option wasn't parsed correctly and simply string comparision could lead to wrong results, e.g. 200 > 1000M
merged branch Tobion/max-filesize-validator (PR #7362)

This PR was merged into the 2.1 branch.

Commits
-------

7216cb0 [Validator] fix showing wrong max file size for upload errors

Discussion
----------

[Validator] fix showing wrong max file size for upload errors

this was because the maxSize option wasn't parsed correctly and simple string comparision could lead to wrong results, e.g. 200 > 1000M

| Q             | A
| ------------- | ---
| Bug fix?      | [yes]
| New feature?  | [no]
| BC breaks?    | [no]
| Deprecations? | [no]
| Tests pass?   | [yes|]
| Fixed tickets | [#6441,#5551]
| License       | MIT

Will apply cleanly to 2.2
[Security] use current request attributes to generate redirect url?
merged branch jfsimon/security-redirect-attributes (PR #7325)

This PR was squashed before being merged into the 2.1 branch (closes #7325).

Commits
-------

6575df6 [Security] use current request attributes to generate redirect url?

Discussion
----------

[Security] use current request attributes to generate redirect url?

Maybe we should consider to use current request attributes to generate the login/logout redirections URL?

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #5080

---------------------------------------------------------------------------

by Seldaek at 2013-03-11T08:33:37Z

Can you explain why?

---------------------------------------------------------------------------

by jfsimon at 2013-03-11T09:30:07Z

@Seldaek let say I prefixed all my URLs with a `{domain}` var (`_locale` for instance), I'd like it to be passed to my redirected request. I guess it could lead to side effects, that's why I tagged this PR `RFC`.

---------------------------------------------------------------------------

by Seldaek at 2013-03-11T09:46:33Z

Fair enough. The main issue I see is that you end up with "garbage" query params in the URL. Any params that was needed by the previous page and not needed by the new one ends up as ?foo=bar in the URL. It's usually not harmful, but not very clean either. I'm not sure what it would take to grab all the params that a route can use, and only copy those over.

---------------------------------------------------------------------------

by jfsimon at 2013-03-11T10:12:49Z

@Seldaek indeed, I didn't think about those query parameters... I'll try to fix this in a simple way this afternoon.

---------------------------------------------------------------------------

by jfsimon at 2013-03-11T14:54:31Z

@Seldaek tell me if what you think of this, it may look like a hack (which wont be acceptable).

---------------------------------------------------------------------------

by Seldaek at 2013-03-11T14:59:39Z

Eh I see. I can't say it's the less hacky thing I ever saw, but it might be alright. I don't think I'm the best person to take this call though.. Let's see what @fabpot thinks.