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.

Sign up for GitHub

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

Jump to bottom

Fix bug where data would be injected inside conditional IE tags #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix bug where data would be injected inside conditional IE tags #5

wants to merge 1 commit into from

Conversation

tomwasd
Copy link
Contributor

@tomwasd tomwasd commented Jan 23, 2016

Hi,

I have a .html file that conditionally loads styles and scripts for IE9, e.g.

<!--[if lt IE 9]>
  <script src="/js/event-polyfill.min.js"></script>
  <script src="/js/html5shiv.min.js"></script>
  <script src="/js/respond.min.js"></script>
  <script src="/js/calc.min.js"></script>
<![endif]-->

Inject data was injecting the data inside the <!--[if lt IE 9]> causing all sorts of issues.

I've changed the code so that rather than jumping in before a <script> tag it just gets added at the end of the <body> tag.

It fixes my problem and I can't see that it would cause any others?

On an unrelated note I've also got a local version of inject-data that doesn't require jQuery - happy to do a PR for that if it's of any use.

@tomwasd
Copy link
Contributor Author

tomwasd commented Jan 23, 2016

I have deferScriptLoading turned on which is moving all my scripts but for the conditional ones (Cheerio doesn't detect them) to the bottom of the page.

Flow Router then sees the conditional script as the first script and injects the code there, within the conditional tags.

I've started to question whether having the data just before </body> (and after all the other scripts) makes a difference so I'll close this for now.

@tomwasd tomwasd closed this Jan 23, 2016
@tomwasd
Copy link
Contributor Author

tomwasd commented Jan 23, 2016

Sorry, flip flopping a bit here!

Having thought about it, I don't think there are any issues with sticking the injected data just before the </body> tag. Although it's a script tag it's not actually executing anything and so it's ordering in the DOM shouldn't matter (I think).

It seems like the best solution to avoid the script tag ending up within conditional tags so I've reopened the PR.

@tomwasd tomwasd reopened this Jan 23, 2016
@jasonk jasonk mentioned this pull request Sep 26, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@tomwasd