[TYPO3-team-core-v5] Comments about commits during my holidays ...

[Sebastian Kurfürst]

Hey,

> May 18 2011 [+BUGFIX] (MVC): Original Request must only be set if there
> is a validation error
> Change-Id: Ic2179e21b97da9ad3cd4e5685f85e961c36edc63
>
> In the commit message you write "Then, the "Speaker Profile" of the
> conference site is correctly filled out again.". Imagine that in the
> changelog / release notes of the FLOW3 distribution – doesn't make a lot
> of sense. Also this changeset cries for a unit test because the logic /
> motivation behind setting the original request could be explained by
> that test and secured at the same time.
You are right. I'll provide a unit test for it. Created issue 
http://forge.typo3.org/issues/27232

> May 16 2011 [+BUGFIX] (MVC): Introduce internal Request Arguments
> Change-Id: Ic0c7a7abbd9cb26de2916acf81bf0a523d9c762a
>
> Why do the internal arguments "__referer" and "__CSRF-TOKEN" have a
> different case? Why not "__csrf-token"? or "__csrfToken"?
Good point, Ill come up with a follow up patch to __csrfToken. Created 
issue http://forge.typo3.org/issues/27233

> *** TYPO3 PACKAGE  *****************************************************
>
> May 17 2011 [+BUGFIX] (UI): Fix JavaScript error when party.name not set
> Change-Id: I120e65aaa3b4b0ec7bbd70a431e6850cb2c8429d
>
> As far as I can see the SetupCommandController _does_ create a Party
> (User) for the created account - so was there maybe something else go wrong?
Yeah, it creates a party for the account; but does not set the name of 
the party. This caused the issue.

> ----------------------------------------------------------------------------------
>
> May 26 2011 [TASK] Fix location of image in error page
> Change-Id: I201362fe27987dcbc8df653db210cf8468a4d8a5
>
> Shouldn't the URI builder create the right link to the images even
> without the "package: 'TYPO3'" argument because "TYPO3" is the current
> package? A bug?
No, the behavior is correct as is, as the following happens:

- Request is built (Controller: foo)
- Dispatcher is called, trying to resolve controller foo
     - foo could not be found -> Error Controller is used instead
-> error controller gets the Request with "Controller: foo" passed in
-> this causes all ViewHelpers inside the Template to use "Controller: 
foo" as current context
-> we need to specify explicitely that we want to use "TYPO3" here.

So, in my opinion, correct behavior, and it only happens in the error 
action.

> *** TYPO3CR PACKAGE  ***************************************************
>
> May 17 2011 [BUGFIX] ProxyNode only persists the newNode if it has not
> been cloned
> Change-Id: I93eaddf0131d1c7ab8ca29108d87976063f0951b
>
> As already mentioned, there are way to many inline comments for my taste.
In this case, I think the inline comments really make sense because they 
explain what environment to expect in which case. However, I'd also be 
fine with moving them to the method comments.

Greets,
Sebastian