youtube / spfjs

A lightweight JS framework for fast navigation and page updates from YouTube
https://youtube.github.io/spfjs/
MIT License
2.24k stars 147 forks source link

Don't inject application/json as javascript #400

Closed tchiotludo closed 8 years ago

tchiotludo commented 8 years ago

We can have script tag with json data

<script id="data" type="application/json">{org: 10, items:["one","two"]}</script>

These tag is not evaluated by browers as javascript if type="application/json" is present. This PR prevent spf to inject these tags as javascript

tchiotludo commented 8 years ago

1) the script is not drop, it was inject in the dom as is without any modification but not send to javascript engine, In my case, I use this kind of script <script type="application/json"> to inject configuration for web component & locale available only on the server side, I can get the dom element and parse it with a simple JSON.parse(document.getElementById("main")) 2) good idea, done 3) also done

tchiotludo commented 8 years ago

I understand the point 1) now. I didn't see the frag.replace function that drop the script ... It's fixed now.

nicksay commented 8 years ago

Thanks for the PR! This is looking good; I've just added a couple minor comments, but after those are fixed, I'll merge this in.

tchiotludo commented 8 years ago

I've just made last cleanup & squash commits

nicksay commented 8 years ago

Hi there! Unfortunately it looks the constant definition was lost in the last update:

src/client/nav/response.js:585: ERROR - element TYPE does not exist on this enum
          var type = spf.nav.response.AttributeRegEx.TYPE.exec(attr);

https://travis-ci.org/youtube/spfjs/builds/131329855

tchiotludo commented 8 years ago

oups, sorry. It's ok now. I've added text/css for style tag that is also concern

nicksay commented 8 years ago

Great, the last thing I'll need is for you to update onto the latest changes from master so I can merge this!

tchiotludo commented 8 years ago

Great news, it's merged

nicksay commented 8 years ago

Would you mind squashing your commits?

tchiotludo commented 8 years ago

I think it's done, I'm not confortable with git rebase ...

nicksay commented 8 years ago

Looks perfect, thanks!