Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.x] Fix bug caused by localisation refactoring #29967

Merged
merged 2 commits into from Sep 12, 2019

Conversation

@marijnkampf
Copy link
Contributor

commented Sep 12, 2019

In Laravel 5.8 calling __(null) would return null. This is no longer the case in Laravel 6.

Calling __(null) now returns a Translator object due to the Translator code refactoring; $this->getFromJson(...) is no longer calling $this->makeReplacements(...).

As this is not mentioned in the Upgrade Guide and the documentation at https://laravel.com/docs/5.7/localization#retrieving-translation-strings states:

If the specified translation string does not exist, the __ function will return the translation string key. So, using the example above, the __ function would return messages.welcome if the translation string does not exist.

I presume this is an unintended bug. This pull request resolves the issue.

Details:
In Laravel 5.8 you could run the following code in a blade:

    @dump(trans(null))  // Returns Translator object in 5.8 and 6.
    @dump(__(null))  // Returns null in 5.8 and Translator object in 6.

    {{ __(null) }} // Runs successfully on 5.8 fails on 6.0 as it expects a string not an object, see full error below.
Facade\Ignition\Exceptions\ViewException
htmlspecialchars() expects parameter 1 to be string, object given (View: resources\views\welcome.blade.php)

If it is intentional the upgrade guide should be updated to put an if statement checking for null for any variables that are translated that could contain null.

marijnkampf added 2 commits Sep 12, 2019
In Laravel 5.8 calling __(null) would return null. Due to the code refactoring __(null) now returns a Translator object. As this is not mentioned in the Upgrade Guide I presume this is an unintended bug. This pull request resolves the issue.

Details:
In Laravel 5.8 you could run the following code in a blade:
    @Dump(trans(null))  // Returns Translator object in 5.8 and 6.
    @Dump(__(null))  // Returns null in 5.8 and Translator object in 6.

    {{ __(null) }} // Runs successfully on 5.8 fails on 6.0 see error below.

Facade\Ignition\Exceptions\ViewException
htmlspecialchars() expects parameter 1 to be string, object given (View: resources\views\welcome.blade.php)

If it is intentional the upgrade guide should be updated to put an if statement checking for null for each translatable variable that could contain null.
@driesvints driesvints changed the title Fix bug caused by localisation refactoring [6.x] Fix bug caused by localisation refactoring Sep 12, 2019
@taylorotwell taylorotwell merged commit 06d22a8 into laravel:6.x Sep 12, 2019
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.