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

An error on servers with PHP 7+ #8636

Merged
merged 1 commit into from Dec 27, 2017

Conversation

Projects
None yet
2 participants
@TMMeilleur
Contributor

TMMeilleur commented Dec 26, 2017

An error occurred on PHP 7+ when attempt to add assets('font-awesome' etc.). It befalls because PHP 7 has a slightly different way of dealing with dynamic variables, which results in a different outcome in some cases.
PHP5 interpretation : $this->{$asset['type']}();
PHP7 interpretation : $this->{$asset}'type';

Questions Answers
Branch? develop
Description? An error occurred on PHP 7+ when attempt to add assets('font-awesome' etc.). It befalls because PHP 7 has a slightly different way of dealing with dynamic variables, which results in a different outcome in some cases.
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? no

Important guidelines


This change is Reviewable

An error on servers with PHP 7+
An error occurred on PHP 7+ when attempt to add assets('font-awesome' etc.). It befalls because PHP 7 has a slightly different way of dealing with dynamic variables, which results in a different outcome in some cases.
PHP5 interpretation : $this->{$asset['type']}();
PHP7 interpretation : $this->{$asset}['type']();
@Quetzacoalt91

Good catch! How can we test that fix?

With a theme with new assets?

@TMMeilleur

This comment has been minimized.

Show comment
Hide comment
@TMMeilleur

TMMeilleur Dec 27, 2017

Contributor

You can add the asset throughout any module with $this->context->controller->requireAssets(array('font-awesome')); in a hookHeader and compare result betwen servers with different php versions

Contributor

TMMeilleur commented Dec 27, 2017

You can add the asset throughout any module with $this->context->controller->requireAssets(array('font-awesome')); in a hookHeader and compare result betwen servers with different php versions

@Quetzacoalt91 Quetzacoalt91 added this to the 1.7.4.0 milestone Dec 27, 2017

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Dec 27, 2017

Member

Thanks for the explanation, I just reproduced the issue.

    public function install()
    {
        [...]
        $this->registerHook('displayHeader');
    }

    public function hookDisplayHeader()
    {
        $this->context->controller->requireAssets(array('font-awesome'));
    }
Member

Quetzacoalt91 commented Dec 27, 2017

Thanks for the explanation, I just reproduced the issue.

    public function install()
    {
        [...]
        $this->registerHook('displayHeader');
    }

    public function hookDisplayHeader()
    {
        $this->context->controller->requireAssets(array('font-awesome'));
    }

@Quetzacoalt91 Quetzacoalt91 merged commit b9b7504 into PrestaShop:develop Dec 27, 2017

2 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
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