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

Mobile album photo view in 13.1.0 not working anymore due to syntax error in footer.tpl in smartpocket theme (with PHP 7.x) #1766

Closed
bigcookie opened this issue Oct 23, 2022 · 14 comments

Comments

@bigcookie
Copy link

bigcookie commented Oct 23, 2022

Hello,

i updated to 13.1.0 recently, but the photo view inside an album is not working anymore. Thrown error in GUI:

Fatal error: Uncaught --> Smarty Compiler: Syntax error in template "file:/var/www/vhosts/xxxx/xxxx/themes/smartpocket/template/footer.tpl" on line 20 "document.cookie = 'screen_size='+jQuery(document).width()+'x'+jQuery(document).height()+';path={isset($COOKIE_PATH) ? $COOKIE_PATH : ''}';" - Unexpected " ? ", expected one of: "}" <-- thrown in /var/www/vhosts/xxxx/xxxxx/include/smarty/libs/sysplugins/smarty_internal_templatecompilerbase.php on line 20

Standard view works fine. Albums are still listed on mobile, but when opening an album, no photos are shown and the error is listed at the bottom of the page

@bigcookie
Copy link
Author

Uploading 52F3062F-BD2E-4886-859F-4FC48147AE96.jpeg…

@bigcookie
Copy link
Author

bigcookie commented Oct 23, 2022

64F451EB-BE05-4A7D-BCE0-816CB0053CEF
5A33AFF6-8305-423A-91E9-2A4963AC13CA
44C02F11-30D4-47DE-A034-A39ED6C7F762

@bigcookie bigcookie changed the title Mobile album view in 13.1.0 nit working anymore Mobile album view in 13.1.0 not working anymore (with PHP 7.0.33) Oct 23, 2022
@bigcookie
Copy link
Author

bigcookie commented Oct 23, 2022

I checked the footer template in
themes/smartpocket/template/footer.tpl
I changed the line 20 from:
document.cookie = 'screen_size='+jQuery(document).width()+'x'+jQuery(document).height()+';path={isset($COOKIE_PATH) ? $COOKIE_PATH : ''}';
to
document.cookie = 'screen_size='+jQuery(document).width()+'x'+jQuery(document).height()+';path={$COOKIE_PATH}';

So I reverted this commit Piwigo/piwigo-smartpocket@1e948e4 . The template error is gone and all photos are showing properly.
Unfortunately I dont know how the syntax should be in a corrected way, so I cannot fix it and commit :-(...

@bigcookie bigcookie changed the title Mobile album view in 13.1.0 not working anymore (with PHP 7.0.33) Mobile album photo view in 13.1.0 not working anymore due to syntax error in footer.tpl in smartpocket theme (with PHP 7.x) Oct 23, 2022
@bigcookie
Copy link
Author

I tried this syntax:
document.cookie = 'screen_size='+jQuery(document).width()+'x'+jQuery(document).height()+';path={$COOKIE_PATH|default:''}';
Not sure if correct, if confirmed to be working for others, this would be a solution...

@erAck
Copy link
Contributor

erAck commented Oct 23, 2022

This part
+';path={isset($COOKIE_PATH) ? $COOKIE_PATH : ''}';
of course is wrong for the levels of single quotes forming string literals to assemble.
I'd guess it should instead be

+';path='{isset($COOKIE_PATH) ? $COOKIE_PATH : ''};

See also https://piwigo.org/forum/viewtopic.php?id=32263

@bigcookie
Copy link
Author

Tested my patch (#1766 (comment)) also on PHP 8 - works so far. I created a PR
Piwigo/piwigo-smartpocket#8

@jamie-marchant
Copy link

Any work around in the meantime?

@erAck
Copy link
Contributor

erAck commented Oct 29, 2022

Sure, try the patch of the pull request mentioned and give feedback whether it works for you. Or try the change of my previous comment. One of them should work. Or replace that line with the old code as mentioned above in #1766 (comment) .

@dj2k
Copy link

dj2k commented Oct 29, 2022

The patch of the pull request works for me and my PHP 7.4.32.

@erAck
Copy link
Contributor

erAck commented Oct 30, 2022

+';path={$COOKIE_PATH|default:''}';

I'm not convinced of that approach, see my comment at the patch Piwigo/piwigo-smartpocket#8 (comment) .

@plegall
Copy link
Member

plegall commented Oct 30, 2022

@erAck @bigcookie have you tested the commit pushed by @MatthieuLP ? Piwigo/piwigo-smartpocket@738dcde

@erAck
Copy link
Contributor

erAck commented Oct 30, 2022

@plegall @MatthieuLP yes that works, thanks!

@jamie-marchant
Copy link

I don't have a phone with this problem but my friend says the site is now working for her.

@bigcookie
Copy link
Author

@plegall the patch from @MatthieuLP works for me.
I agree, that I didnt understand to replace a missing string with an empty one. I just tried to fix the syntax error. The new patch looks better to me.
I will cancel my PR.

@plegall plegall closed this as completed Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants