I have tested this pull request with Community Builder 1.7.1 and it fixes the bug that was introduced by the commit a962ad7 that was not an exact reversal of pull request #566.

Commented on this google thread too:
http://groups.google.com/group/joomla-dev-general/browse_thread/thread/8e7ac39b458f8037/0c49392e9ace754f

Unit testing complete. There were 0 failures and 0 errors from 1971 tests and 11146 assertions.
Checkstyle analysis reported 199 warnings and 0 errors.

I would vote for a "assignment in condition" sniff ;)

Agree elkuku, really wondering about that one ... and why we don't just fix that? :)
This diff shows the code that we are restoring

beat@2aee17e#libraries/joomla/application/component/helper.php

So no, in fact this is not a bug introduced by restoring the code, it was there long before.

if (($path = JApplicationHelper::getPath('toolbar')) && $app->isAdmin())

{

  // Get the task again, in case it has changed

  $task = JRequest::getString('task');
  // Make the toolbar
  include_once $path;
}

So it should go back to this exactly and then if people want other changes that's great but a separate issue.

Elin,

eikuku's comment was joking (notice: ;) ):

The parenthesis DO MATTER. Sorry. Explaining it basic programming way here:

Your change: a962ad7 :

• if ($path = JApplicationHelper::getPath('toolbar') && $app->isAdmin())

Is not same as what was removed by #566 :

• if (($path = JApplicationHelper::getPath('toolbar')) && $app->isAdmin())

Notice the missing ( ) around the assignment that was in the if statement ?

your change means:

• if ($path = ( JApplicationHelper::getPath('toolbar') && $app->isAdmin() ) )
  and original was:
• if ($path = ( JApplicationHelper::getPath('toolbar') ) && $app->isAdmin() )

See that your changes gives a logical boolean true/false result instead of a string with path in $path ?

That's one of the many common errors made by programmers when there are assignments inside "if" statements.
Assignments inside "if" statements are not considered to be safe and clean programing, because they mix 2 different actions in a single line, and mix-up people, exactly like you got mixed up.

You can fix the bug you introduced by putting back exactly what was there with the missing paranthesies....

...or do it the clean way I proposed, which is 100% equivalent, and won't mix the next php programmer touching that line + won't generate a php lint warning for bad practice.

I do not care which way you revert #566 to what IT was DOING, as long as it's doing it. If you want to keep it old unclean way, up to you.

@beat Actually I was pretty serious, but always with a little ;)

There is already a Squiz sniff for "assignment in condition" but it seems to trigger also ternaries - working on it..

@pasamio Thank you for the merge ! :)

@elkuku This is not the place to discuss coding style (google for "assignment in condition" will give plenty of discussions and explanations why that is prone to leading to errors like the one that Elin did by forgetting the parenthesizes and why most IDEs flag them with warnings). Feel free to open a new thread in google groups if needed. ;)

wow @beat, seems you got me pretty wrong (:() I am totally in favor of the "elimination" of any "assignment in condition" (no need to ask google why..). I believe it's very bad style and it always bugs me whenever I find it. My current IDE also marks it red..
I would love to see such a sniff being implemented to avoid further errors (like the one at hand).
Apologies for any misunderstanding, as english is also not my native language.

As for the "place to discuss" - of course, you're right - it was meant more as a "little site note" while we are on it ;)

@elkuku No worries, I got the meaning of your first comment right then, interpreting it the obvious way, even if I misinterpreted your "sniff" as a "tear" because of the ;) following it instead of the sniff program :D . It was Elin's comment which didn't understand the bug that she introduced and that was voting to restore the bad assignment in condition "exactly like it was", followed by the "i was serious" in reply after Elin's comment that raised doubts about the obvious that every non-beginner programmer konws.

Case closed :)