Extra body tags in the rendered html. #8

Closed
Fishes opened this Issue Dec 5, 2014 · 14 comments

Comments

Projects
None yet
5 participants
@Fishes

Fishes commented Dec 5, 2014

When a directive is added the added html tags are surrounded in an extra body tag. And preceded by an extra head tag.

@ClemMakesApps

This comment has been minimized.

Show comment
Hide comment
@ClemMakesApps

ClemMakesApps Dec 5, 2014

I fixed this issue in my fork: https://github.com/ClemMakesApps/winstore-jscompat
I would submit a pull request but I am not sure if this breaks anything else. I have only tested it in a few applications.

I fixed this issue in my fork: https://github.com/ClemMakesApps/winstore-jscompat
I would submit a pull request but I am not sure if this breaks anything else. I have only tested it in a few applications.

@panarasi

This comment has been minimized.

Show comment
Hide comment
@panarasi

panarasi Dec 6, 2014

Contributor

I think a pull request would be great. Can you also sign the Contributor's License Agreement (CLA) for the project here - https://cla.msopentech.com/ ?

Contributor

panarasi commented Dec 6, 2014

I think a pull request would be great. Can you also sign the Contributor's License Agreement (CLA) for the project here - https://cla.msopentech.com/ ?

@ClemMakesApps

This comment has been minimized.

Show comment
Hide comment
@ClemMakesApps

ClemMakesApps Dec 7, 2014

Sure. I'll definitely do that. My company is busy during the holidays, so I probably won't be able to submit a pull request till January.

Sure. I'll definitely do that. My company is busy during the holidays, so I probably won't be able to submit a pull request till January.

@wbreza

This comment has been minimized.

Show comment
Hide comment
@wbreza

wbreza Jan 6, 2015

Thanks for fixing this. I was going crazy trying to find out where the extra body tags were coming from. This was screwing up any component JS that was referencing document.body as it was pointing to the extra (empty) body tag. I'm looking forward to seeing this PR get into the master source.

wbreza commented Jan 6, 2015

Thanks for fixing this. I was going crazy trying to find out where the extra body tags were coming from. This was screwing up any component JS that was referencing document.body as it was pointing to the extra (empty) body tag. I'm looking forward to seeing this PR get into the master source.

@xirzec xirzec self-assigned this Jan 22, 2015

xirzec added a commit that referenced this issue Jan 22, 2015

Fix #8
The dynamic document we create to pre-process HTML will automatically create wrapping document, head, and body elements when HTML is passed in. This resulted in extra <head> and <body> tags getting created when calling innerHTML on elements that are not document elements.
@xirzec

This comment has been minimized.

Show comment
Hide comment
@xirzec

xirzec Jan 22, 2015

Member

I created a fix in #11. @wbreza @Fishes can you give it a try?

Member

xirzec commented Jan 22, 2015

I created a fix in #11. @wbreza @Fishes can you give it a try?

@Fishes

This comment has been minimized.

Show comment
Hide comment
@Fishes

Fishes Jan 23, 2015

The fix works for the specified problem.
But in my project I include WinJS components and these fail with this fix.
Inititalizing the appBar and adding buttons fails at line 133:13 with the error Invallid pointer.
Sorry

Fishes commented Jan 23, 2015

The fix works for the specified problem.
But in my project I include WinJS components and these fail with this fix.
Inititalizing the appBar and adding buttons fails at line 133:13 with the error Invallid pointer.
Sorry

@wbreza

This comment has been minimized.

Show comment
Hide comment
@wbreza

wbreza Jan 23, 2015

Looks like it works for elements that get injected under the body, but it doesn't work when elements are dynamically injected into the HEAD of the page.

For example, when using this with angular, the default style block that angular attempts to injects into the head of the page does not work.

I've created a pull request that fixed it for my case. Take a look and let me know if it looks good.

wbreza commented Jan 23, 2015

Looks like it works for elements that get injected under the body, but it doesn't work when elements are dynamically injected into the HEAD of the page.

For example, when using this with angular, the default style block that angular attempts to injects into the head of the page does not work.

I've created a pull request that fixed it for my case. Take a look and let me know if it looks good.

@xirzec

This comment has been minimized.

Show comment
Hide comment
@xirzec

xirzec Jan 23, 2015

Member

@Fishes - Looks like there is a problem in AppBar specifically: winjs/winjs#899

@wbreza - How would that work if you are calling innerHTML on a documentElement? Your code will always strip out <body> and <head> tags.

Member

xirzec commented Jan 23, 2015

@Fishes - Looks like there is a problem in AppBar specifically: winjs/winjs#899

@wbreza - How would that work if you are calling innerHTML on a documentElement? Your code will always strip out <body> and <head> tags.

@wbreza

This comment has been minimized.

Show comment
Hide comment
@wbreza

wbreza Jan 23, 2015

@xirzec - I haven't tested that case. I'll update again when I have some time to test it out.

wbreza commented Jan 23, 2015

@xirzec - I haven't tested that case. I'll update again when I have some time to test it out.

@xirzec

This comment has been minimized.

Show comment
Hide comment
@xirzec

xirzec Jan 23, 2015

Member

@wbreza - Can you give an example of what angular is doing to inject styles? I updated #11 with a guess, but I wasn't sure if they were injecting a tag like <style> that document wants to group in <head> but they wanted to put in the <body> or something.

Member

xirzec commented Jan 23, 2015

@wbreza - Can you give an example of what angular is doing to inject styles? I updated #11 with a guess, but I wasn't sure if they were injecting a tag like <style> that document wants to group in <head> but they wanted to put in the <body> or something.

@xirzec

This comment has been minimized.

Show comment
Hide comment
@xirzec

xirzec Jan 23, 2015

Member

@Fishes - I was wrong about it being an AppBar problem, the problem was actually because AppBar sets an empty string as innerHTML. I updated #11 to work around that as well.

Member

xirzec commented Jan 23, 2015

@Fishes - I was wrong about it being an AppBar problem, the problem was actually because AppBar sets an empty string as innerHTML. I updated #11 to work around that as well.

@wbreza

This comment has been minimized.

Show comment
Hide comment
@wbreza

wbreza Jan 23, 2015

@xirzec - Take a look at the last line of Angular and you'll see they are finding the 'head' element and prepending a style block.

wbreza commented Jan 23, 2015

@xirzec - Take a look at the last line of Angular and you'll see they are finding the 'head' element and prepending a style block.

@xirzec

This comment has been minimized.

Show comment
Hide comment
@xirzec

xirzec Jan 23, 2015

Member

@wbreza Thanks for the pointer. Yep, they're inserting a <style> tag into a <div>'s innerHTML.

Member

xirzec commented Jan 23, 2015

@wbreza Thanks for the pointer. Yep, they're inserting a <style> tag into a <div>'s innerHTML.

@xirzec

This comment has been minimized.

Show comment
Hide comment
@xirzec

xirzec Jan 23, 2015

Member

@wbreza Alright! I think I have something workable in #11 - it works with Angular, it works with AppBar, it works with a simple div, it works with a new document element.

Member

xirzec commented Jan 23, 2015

@wbreza Alright! I think I have something workable in #11 - it works with Angular, it works with AppBar, it works with a simple div, it works with a new document element.

@xirzec xirzec closed this in 5c0039a Jan 24, 2015

xirzec added a commit that referenced this issue Jan 24, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment