Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Extra body tags in the rendered html. #8

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

Extra body tags in the rendered html. #8

Fishes opened this issue Dec 5, 2014 · 14 comments

Comments

@Fishes
Copy link

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
Copy link

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
Copy link
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
Copy link

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
Copy link

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
@xirzec
Fix #8
d04cc75 
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
Copy link
Member

xirzec commented Jan 22, 2015

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

@Fishes
Copy link
Author

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
Copy link

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.

@ClemMakesApps ClemMakesApps mentioned this issue Jan 23, 2015
Closed
@xirzec
Copy link
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
Copy link

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
Copy link
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
Copy link
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
Copy link

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
Copy link
Member

xirzec commented Jan 23, 2015

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

@xirzec
Copy link
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 as completed in 5c0039a Jan 24, 2015
xirzec added a commit that referenced this issue Jan 24, 2015
@xirzec
Merge pull request #11 from MSOpenTech/fixExtraTags
1c23c04 
Fix #8
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@xirzec @panarasi @Fishes @ClemMakesApps @wbreza