06599: All old custom templates are incompatible with Limesurvey 2 - LimeSurvey bugs, patches and feature requests

(20851)
Mazi (developer)
2012-09-27 15:57

Denis, Carsten is very busy so I'm assigning this one to you. Let me know if you have any questions.

(20852)
Mazi (developer)
2012-09-27 16:02

You can see the broken conditions and the broken progress bar here. At this demo survey, a following question should show up if you select "yes":

- 1.92 "basic" template: http://limesurvey-templates.com/20/index.php/survey/index/sid/657996/newtest/Y/lang/en [^]

- 2.0 "basic" shipped template: http://limesurvey-templates.com/20/index.php/survey/index/sid/382536/newtest/Y/lang/en [^]

(20853)
DenisChenu (developer)
2012-09-27 16:07
edited on: 2012-09-27 16:13

1.8 -> 1.9 : Condition don't work (question.pstpl + {QUESTION_ESSENTIALS})
1.91 -> 1.92 : hide_tip class), if you don't add some line : then hide-tip attribute don't work.

There a lot of incompatibility each time we make a sub update (1.7,1.8, ....), here it's a major update 1 -> 2.

Thin we can have same think for 2.0, and now : we can start to use some PHP script to compress js file and css files.

I can do a
str_replace("<script type="text/javascript" src="{TEMPLATEURL}template.js"/></script>","{TEMPLATEJS}" ) for all template, but it's for 1.92 compatibility and there are other modification to do to have compatibilty between 1.92 and 2.0 (some css for example). And it's an instruction more for compatible template.

Here, to update template: just 1 line to change.
{TEMPLATEURL}template.js -> {TEMPLATEJS}

If you remind the 1.90 at the forum, we have a lot of user with : conditoon do not work, becaus the question.pstpl.

Denis

(20854)
tpartner (developer)
2012-09-27 16:16

I think we should do the string replacement or some other "automatic" solution for existing custom templates - there will be a LOT of them.

(20855)
jonbyrne (reporter)
2012-09-27 16:18

We have this problem here too. It would nice if this could be fixed as the rest of 2.0 seems very good.

Jon

(20856)
Mazi (developer)
2012-09-27 16:30

Thanks for your feedback, Denis.

I absolutely understand your point that between certain releases, incompatibilities may show up.
BUT in this case, this is really a major issue which we should be able to solve for the benefit of all the users who will be starting to switch to Limesurvey 2 soon and use their own templates.
As Tony has pointed out, there will be LOTS of users running into this issue.

(20858)
mdekker (developer)
2012-09-27 17:09

In general I think incompatible template changes should get their own place in the release notes. Wordpress for example always tells what was changed for template designers and plugin developers.

For this specific change... Expression Manager introduced the same kind of issues when the validation css changed and some default block had to be added to all templates.
http://docs.limesurvey.org/Upgrade+hints+for+version+1.92 [^]
Don't know if that was linked somewhere... had to search a lot to find it. So I would vote for document and make it easy to find :)

(20859)
DenisChenu (developer)
2012-09-27 17:19

I search for an:

str_replace("<script type="text/javascript" src="{TEMPLATEURL}template.js"/></script>","{TEMPLATEJS}" ) in upload/template/*/startpage.pstpl before apply it allways.

Denis

(20860)
DenisChenu (developer)
2012-09-27 19:38

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=9568 [^]

(20861)
DenisChenu (developer)
2012-09-27 19:39

Docs:
http://docs.limesurvey.org/The+template+editor#CSS_and_Javascript [^]
http://docs.limesurvey.org/The+template+editor#Standard_Pages [^] (Note, look to put elsewhere)

(20862)
DenisChenu (developer)
2012-09-27 19:40

Mazi: can you look if it's OK for the update with the last GIT version.

(20871)
Mazi (developer)
2012-09-28 10:40

Thanks for looking into this, Denis.

Unfortunately the fix only adds the missing {TEMPLATEJS} by editing the template files during update.

This is a problem for several reasons:

1. Lots of current LS 1.9x users set up LS2 as a test system and then later switch to the new version by copying surveys and templates. In this case the templates will not be updated.

2. There are users which run several Limesurvey installations and copy surveys and templates between them. Again, the templates will not be updated by this fix.

3. Some users, especially admins of larger organisations, have lots of templates stored on their local PC and import them when necessary. In this case, the template update will also not take place.

Though your fix is great code-wise, there are some more situations where your fix might not be sufficient.
Therefor I suggest a solution which improves the replacement function. It shouldn't be that difficult to check for the {TEMPLATEJS} placeholder when processing (the startpage.pstpl of) a template. if it is not found at the current template we simply skip the replacement and instead include the necessary JS files the way we did it for Limesurvey 1.9x.

This way all the problems mentioned above will be solved, we need no new DB version number and we also wouldn't need to take care of extending the DB setup and update procedure.

Do you think you could implement it this way?

Thanks!

(20872)
DenisChenu (developer)
2012-09-28 10:47

I i test if TEMPLATEJS are in startpage.pstpl, then it can be adding 2 times:
in startpage.pstpl and in endpage.pstpl.

Some user can want to have some survey witjout javascript etc ....

The best wayi sto make an
str_replace("<script type="text/javascript" src="{TEMPLATEURL}template.js"/></script>","{TEMPLATEJS}" ) in upload/template/*/startpage.pstpl
Before replace {TEMPLATEJS} in replacement.php.

(20873)
c_schmitz (administrator)
2012-09-28 10:58
edited on: 2012-09-28 10:59

Shnoulle,

I talked to Mazi, discussed this and we both think that your solution is fine. I like this much better than replacing this on the fly again and again during runtime. Two points are missing though.

* In the template import the same replacement should be done.
* Edit the upgrade instructions and make clear that the templates need to be copied back to the installation _before_ the installation is started/the database upgarde is run (and so the templates are updated)

Can you add these missing points, please?

Great work, btw.

(20874)
DenisChenu (developer)
2012-09-28 11:06

* In the template import the same replacement should be done.,

Oh yeah ! GREAT IDEA !!!

Before this idea, i think of a "error line" at template editor:
- This template seem missed {TEMPLATEJS}
- This template seem misses question.pstpl
Please see the documentation of LimeSurvey.

Denis

(20875)
tpartner (developer)
2012-09-28 11:41

Yeah, great work Denis.

But just so I'm clear on the end result:

- All templates that are in the installation that is being upgraded and all imported templates will be automatically modified.

- Templates that are simply uploaded to the upload/templates directory by those people who don't use the template editor will not be updated and will need to be modified for use.

(20876)
Mazi (developer)
2012-09-28 11:43

I think you are right, Tony... but that brings us to another problem:

What about templates being uploaded by FTP instead of using the template editor?

(20884)
Fred (reporter)
2012-09-28 16:45

Have the same problem here.

(20885)
c_schmitz (administrator)
2012-09-28 16:48

Well, upload by FTP was never supported by the application, anyway. In general we have to assume that people use the import function.

(20886)
DenisChenu (developer)
2012-09-28 18:22
edited on: 2012-09-28 18:24

Something i don't understand,

If you upload via FTP, you can't take time to change one line before upload ?

Denis
Tony : user who use sftp or some other access have the ability to replace one line in startpage.pstpl.

And again, it's not the only think to change betwwen template.
This one is great : you see it very quickliy, other are question specific, and maybe it's not you seeing it, but your customer.

Denis

(20887)
Mazi (developer)
2012-09-28 22:38

You are right, Denis. We can't take care of users uploading templates via FTP. In this case we should point them to the manual.

So if we take care about template import (fix needed) and have another check when updating to LS2 (already implemented), I think that is all we can do for now.

(20978)
DenisChenu (developer)
2012-10-02 13:52

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=9610 [^]

(21008)
DenisChenu (developer)
2012-10-03 19:09

Reminder sent to: Mazi, tpartner

Can you look if it's OK please ?

(21010)
tpartner (developer)
2012-10-03 20:27

Looks fine to me Denis, thanks.

(21012)
c_schmitz (administrator)
2012-10-03 22:12

You forgot to raise the DBVersion on the .sql files but I did that for you now ;). Thank you for your work!

(21016)
DenisChenu (developer)
2012-10-04 07:29

At install, yes, I see that .

Thanks to you

(21025)
Mazi (developer)
2012-10-04 11:25

I can confirm that Denis' fix works great!

Thanks a lot for solving this issue, Denis.

(21082)
c_schmitz (administrator)
2012-10-05 15:26

New version 2.00+ build 121005 released.

Related Changesets
LimeSurvey: master 37f717e0 Timestamp: 2012-09-27 10:37:50 Author: DenisChenu Committer: Denis Chenu [ Details ] [ Diff ]	Fixed issue 06599: Old template incompatibilty, need {TEMPLATEJS} Dev : update db version to replace template.js line with {TEMPLATEJS} Dev: fix error_view in response browsing for inactive survey
	mod - application/config/version.php	[ Diff ] [ File ]
	mod - application/controllers/admin/responses.php	[ Diff ] [ File ]
	mod - application/helpers/update/updatedb_helper.php	[ Diff ] [ File ]

LimeSurvey: master e07579c5 Timestamp: 2012-10-02 04:51:34 Author: DenisChenu Committer: Denis Chenu [ Details ] [ Diff ]	Fixed issue 06599 : Old custom templates incompatibility : replace template.js by {TEMPLATEJS} when upload Dev: add a private function _templateFixes($templatename) Dev : update release_notes.txt , to do : update docs.limesurvey.org
	mod - application/controllers/admin/templates.php	[ Diff ] [ File ]
	mod - application/views/admin/templates/importuploaded_view.php	[ Diff ] [ File ]
	mod - docs/release_notes.txt	[ Diff ] [ File ]

Notes
(20851) Mazi (developer) 2012-09-27 15:57	Denis, Carsten is very busy so I'm assigning this one to you. Let me know if you have any questions.

(20852) Mazi (developer) 2012-09-27 16:02	You can see the broken conditions and the broken progress bar here. At this demo survey, a following question should show up if you select "yes": - 1.92 "basic" template: http://limesurvey-templates.com/20/index.php/survey/index/sid/657996/newtest/Y/lang/en [^] - 2.0 "basic" shipped template: http://limesurvey-templates.com/20/index.php/survey/index/sid/382536/newtest/Y/lang/en [^]

(20853) DenisChenu (developer) 2012-09-27 16:07 edited on: 2012-09-27 16:13	1.8 -> 1.9 : Condition don't work (question.pstpl + {QUESTION_ESSENTIALS}) 1.91 -> 1.92 : hide_tip class), if you don't add some line : then hide-tip attribute don't work. There a lot of incompatibility each time we make a sub update (1.7,1.8, ....), here it's a major update 1 -> 2. Thin we can have same think for 2.0, and now : we can start to use some PHP script to compress js file and css files. I can do a str_replace("<script type="text/javascript" src="{TEMPLATEURL}template.js"/></script>","{TEMPLATEJS}" ) for all template, but it's for 1.92 compatibility and there are other modification to do to have compatibilty between 1.92 and 2.0 (some css for example). And it's an instruction more for compatible template. Here, to update template: just 1 line to change. {TEMPLATEURL}template.js -> {TEMPLATEJS} If you remind the 1.90 at the forum, we have a lot of user with : conditoon do not work, becaus the question.pstpl. Denis

(20854) tpartner (developer) 2012-09-27 16:16	I think we should do the string replacement or some other "automatic" solution for existing custom templates - there will be a LOT of them.

(20855) jonbyrne (reporter) 2012-09-27 16:18	We have this problem here too. It would nice if this could be fixed as the rest of 2.0 seems very good. Jon

(20856) Mazi (developer) 2012-09-27 16:30	Thanks for your feedback, Denis. I absolutely understand your point that between certain releases, incompatibilities may show up. BUT in this case, this is really a major issue which we should be able to solve for the benefit of all the users who will be starting to switch to Limesurvey 2 soon and use their own templates. As Tony has pointed out, there will be LOTS of users running into this issue.

(20858) mdekker (developer) 2012-09-27 17:09	In general I think incompatible template changes should get their own place in the release notes. Wordpress for example always tells what was changed for template designers and plugin developers. For this specific change... Expression Manager introduced the same kind of issues when the validation css changed and some default block had to be added to all templates. http://docs.limesurvey.org/Upgrade+hints+for+version+1.92 [^] Don't know if that was linked somewhere... had to search a lot to find it. So I would vote for document and make it easy to find :)

(20859) DenisChenu (developer) 2012-09-27 17:19	I search for an: str_replace("<script type="text/javascript" src="{TEMPLATEURL}template.js"/></script>","{TEMPLATEJS}" ) in upload/template/*/startpage.pstpl before apply it allways. Denis

(20860) DenisChenu (developer) 2012-09-27 19:38	Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=9568 [^]

(20861) DenisChenu (developer) 2012-09-27 19:39	Docs: http://docs.limesurvey.org/The+template+editor#CSS_and_Javascript [^] http://docs.limesurvey.org/The+template+editor#Standard_Pages [^] (Note, look to put elsewhere)

(20862) DenisChenu (developer) 2012-09-27 19:40	Mazi: can you look if it's OK for the update with the last GIT version.

(20871) Mazi (developer) 2012-09-28 10:40	Thanks for looking into this, Denis. Unfortunately the fix only adds the missing {TEMPLATEJS} by editing the template files during update. This is a problem for several reasons: 1. Lots of current LS 1.9x users set up LS2 as a test system and then later switch to the new version by copying surveys and templates. In this case the templates will not be updated. 2. There are users which run several Limesurvey installations and copy surveys and templates between them. Again, the templates will not be updated by this fix. 3. Some users, especially admins of larger organisations, have lots of templates stored on their local PC and import them when necessary. In this case, the template update will also not take place. Though your fix is great code-wise, there are some more situations where your fix might not be sufficient. Therefor I suggest a solution which improves the replacement function. It shouldn't be that difficult to check for the {TEMPLATEJS} placeholder when processing (the startpage.pstpl of) a template. if it is not found at the current template we simply skip the replacement and instead include the necessary JS files the way we did it for Limesurvey 1.9x. This way all the problems mentioned above will be solved, we need no new DB version number and we also wouldn't need to take care of extending the DB setup and update procedure. Do you think you could implement it this way? Thanks!

(20872) DenisChenu (developer) 2012-09-28 10:47	I i test if TEMPLATEJS are in startpage.pstpl, then it can be adding 2 times: in startpage.pstpl and in endpage.pstpl. Some user can want to have some survey witjout javascript etc .... The best wayi sto make an str_replace("<script type="text/javascript" src="{TEMPLATEURL}template.js"/></script>","{TEMPLATEJS}" ) in upload/template/*/startpage.pstpl Before replace {TEMPLATEJS} in replacement.php.

(20873) c_schmitz (administrator) 2012-09-28 10:58 edited on: 2012-09-28 10:59	Shnoulle, I talked to Mazi, discussed this and we both think that your solution is fine. I like this much better than replacing this on the fly again and again during runtime. Two points are missing though. * In the template import the same replacement should be done. * Edit the upgrade instructions and make clear that the templates need to be copied back to the installation _before_ the installation is started/the database upgarde is run (and so the templates are updated) Can you add these missing points, please? Great work, btw.

(20874) DenisChenu (developer) 2012-09-28 11:06	* In the template import the same replacement should be done., Oh yeah ! GREAT IDEA !!! Before this idea, i think of a "error line" at template editor: - This template seem missed {TEMPLATEJS} - This template seem misses question.pstpl Please see the documentation of LimeSurvey. Denis

(20875) tpartner (developer) 2012-09-28 11:41	Yeah, great work Denis. But just so I'm clear on the end result: - All templates that are in the installation that is being upgraded and all imported templates will be automatically modified. - Templates that are simply uploaded to the upload/templates directory by those people who don't use the template editor will not be updated and will need to be modified for use.

(20876) Mazi (developer) 2012-09-28 11:43	I think you are right, Tony... but that brings us to another problem: What about templates being uploaded by FTP instead of using the template editor?

(20884) Fred (reporter) 2012-09-28 16:45	Have the same problem here.

(20885) c_schmitz (administrator) 2012-09-28 16:48	Well, upload by FTP was never supported by the application, anyway. In general we have to assume that people use the import function.

(20886) DenisChenu (developer) 2012-09-28 18:22 edited on: 2012-09-28 18:24	Something i don't understand, If you upload via FTP, you can't take time to change one line before upload ? Denis Tony : user who use sftp or some other access have the ability to replace one line in startpage.pstpl. And again, it's not the only think to change betwwen template. This one is great : you see it very quickliy, other are question specific, and maybe it's not you seeing it, but your customer. Denis

(20887) Mazi (developer) 2012-09-28 22:38	You are right, Denis. We can't take care of users uploading templates via FTP. In this case we should point them to the manual. So if we take care about template import (fix needed) and have another check when updating to LS2 (already implemented), I think that is all we can do for now.

(20978) DenisChenu (developer) 2012-10-02 13:52	Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=9610 [^]

(21008) DenisChenu (developer) 2012-10-03 19:09	Reminder sent to: Mazi, tpartner Can you look if it's OK please ?

(21010) tpartner (developer) 2012-10-03 20:27	Looks fine to me Denis, thanks.

(21012) c_schmitz (administrator) 2012-10-03 22:12	You forgot to raise the DBVersion on the .sql files but I did that for you now ;). Thank you for your work!

(21016) DenisChenu (developer) 2012-10-04 07:29	At install, yes, I see that . Thanks to you

(21025) Mazi (developer) 2012-10-04 11:25	I can confirm that Denis' fix works great! Thanks a lot for solving this issue, Denis.

(21082) c_schmitz (administrator) 2012-10-05 15:26	New version 2.00+ build 121005 released.

Issue History
Date Modified	Username	Field	Change
2012-09-27 15:49	Mazi	New Issue
2012-09-27 15:49	Mazi	Status	new => assigned
2012-09-27 15:49	Mazi	Assigned To	=> DenisChenu
2012-09-27 15:56	Mazi	Description Updated	View Revisions
2012-09-27 15:56	Mazi	Steps to Reproduce Updated	View Revisions
2012-09-27 15:56	Mazi	Additional Information Updated	View Revisions
2012-09-27 15:56	Mazi	File Added: limesurvey_survey_456431.lss
2012-09-27 15:57	Mazi	Note Added: 20851
2012-09-27 16:02	Mazi	Note Added: 20852
2012-09-27 16:07	DenisChenu	Note Added: 20853
2012-09-27 16:13	DenisChenu	Note Edited: 20853	View Revisions
2012-09-27 16:16	tpartner	Note Added: 20854
2012-09-27 16:18	jonbyrne	Note Added: 20855
2012-09-27 16:30	Mazi	Note Added: 20856
2012-09-27 17:09	mdekker	Note Added: 20858
2012-09-27 17:19	DenisChenu	Note Added: 20859
2012-09-27 19:38	DenisChenu	Changeset attached	=> LimeSurvey master 37f717e0
2012-09-27 19:38	DenisChenu	Note Added: 20860
2012-09-27 19:38	DenisChenu	Resolution	open => fixed
2012-09-27 19:39	DenisChenu	Note Added: 20861
2012-09-27 19:40	DenisChenu	Note Added: 20862
2012-09-27 19:40	DenisChenu	Assigned To	DenisChenu => Mazi
2012-09-27 19:40	DenisChenu	Status	assigned => feedback
2012-09-28 10:40	Mazi	Note Added: 20871
2012-09-28 10:40	Mazi	Status	feedback => assigned
2012-09-28 10:40	Mazi	Assigned To	Mazi => DenisChenu
2012-09-28 10:47	DenisChenu	Note Added: 20872
2012-09-28 10:58	c_schmitz	Note Added: 20873
2012-09-28 10:58	c_schmitz	Note Edited: 20873	View Revisions
2012-09-28 10:59	c_schmitz	Note Edited: 20873	View Revisions
2012-09-28 11:06	DenisChenu	Note Added: 20874
2012-09-28 11:41	tpartner	Note Added: 20875
2012-09-28 11:43	Mazi	Note Added: 20876
2012-09-28 16:45	Fred	Note Added: 20884
2012-09-28 16:48	c_schmitz	Note Added: 20885
2012-09-28 18:22	DenisChenu	Note Added: 20886
2012-09-28 18:24	DenisChenu	Note Edited: 20886	View Revisions
2012-09-28 22:38	Mazi	Note Added: 20887
2012-10-02 13:52	DenisChenu	Changeset attached	=> LimeSurvey master e07579c5
2012-10-02 13:52	DenisChenu	Note Added: 20978
2012-10-03 19:09	DenisChenu	Note Added: 21008
2012-10-03 20:27	tpartner	Note Added: 21010
2012-10-03 22:12	c_schmitz	Note Added: 21012
2012-10-03 22:12	c_schmitz	Status	assigned => resolved
2012-10-03 22:12	c_schmitz	Fixed in Version	=> 2.00+
2012-10-04 07:29	DenisChenu	Note Added: 21016
2012-10-04 11:25	Mazi	Note Added: 21025
2012-10-05 15:26	c_schmitz	Note Added: 21082
2012-10-05 15:26	c_schmitz	Status	resolved => closed

Relationships