BUGFIX double requirements #143

Closed
wants to merge 1 commit into
from

2 participants

@miiihi

BUGFIX Having <header tag in HTML causes requirements to double. See http://www.silverstripe.org/general-questions/show/17524

@chillu
SilverStripe Ltd. member

Hey there, I can't reproduce this problem via unit tests (checked trunk and post-2.4), see https://gist.github.com/1512331. Also tried to insert

tags in standard blackcandy theme, the JS is correctly included just before the closing body tag. Do you have a sample SS template where it fails?

@miiihi

To reproduce: add < header>foo< /header> to templates/Layout/Page.ss
or add "< head>< /head>" to the same file. My patch fixes only the first case.
(note space after < is Markup workaround only)

Reason: SSViewer includes Requeirements for each subtemplate as well, but having protector in Requirements::includeInHTML(639) that only includes requierements if it founds '</head' string in content. So if you have '</head' string in any subtemplate it would include requirements twice (or more...)

My patch is only a quick fix, correct fix would be not calling Requirements::includeInHTML for subtemplates and always including Requierements for main template (regardless of '</head' string).

@chillu
SilverStripe Ltd. member

I still can't reproduce it in practice, but see the problem in the code - merged in 565e2ab and b1dae47 - thanks, and merry christmas to you!

@chillu chillu closed this Dec 23, 2011
@AntiSol AntiSol added a commit to petpack/sapphire that referenced this pull request Nov 17, 2015
@AntiSol AntiSol github #143: switch default database engine to InnoDB ff381d7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment