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.

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

fix(platform-server): fix an exception when HostListener('window:scroll') is used on the server #15019

Merged
merged 1 commit into from
Mar 15, 2017

Conversation

vikerman
Copy link
Contributor

@vikerman vikerman commented Mar 8, 2017

Fixes #15000

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
Trying to bootstrap when a component has @HostListener("window:scroll") causes an exception on the server

What is the new behavior?
No exception

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[X] No

@vikerman vikerman added the action: merge The PR is ready for merge by the caretaker label Mar 9, 2017
@FrozenPandaz
Copy link
Contributor

Do you want me to fix title, it can now query the document.head

for (let i = 0; i < docElement.childNodes.length; i++) {
let node = docElement.childNodes[i];
if (node.name === 'head') {
doc['head'] = node;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be a getter for the head node

doc['head'] = node;
}
if (node.name === 'body') {
doc['body'] = node;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be a getter for the body node

@PatrickJS
Copy link
Member

PatrickJS commented Mar 9, 2017

you're going to run into the same problem as universal 2.1.x has. There will be a case when you're referencing a detached node if the document was changed after creation.

PS: You should include .title where the bug I mentioned becomes more relevant

if (node.name === 'head') {
doc['head'] = node;
}
if (node.name === 'body') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else if / switch

@jimjim2a
Copy link

There is another issue when the html string has a Doctype, it appears as the first node of the parsed document. Therefore the head and body nodes aren't retrieved properly.

@PatrickJS
Copy link
Member

@jimjim2a is correct this is also why we had weird searching logic for the rootNode
https://github.com/angular/universal/blob/master/modules/platform-node/node-document.ts

@@ -31,7 +31,19 @@ function _notImplemented(methodName: string) {
* Parses a document string to a Document object.
*/
export function parseDocument(html: string) {
return parse5.parse(html, {treeAdapter: parse5.treeAdapters.htmlparser2});
let doc = parse5.parse(html, {treeAdapter: parse5.treeAdapters.htmlparser2});
let docElement = doc.childNodes[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this interact with a <!doctype>?

@chuckjaz
Copy link
Contributor

This PR now needs to be rebased.

@vikerman
Copy link
Contributor Author

@chuckjaz - Rebased

@chuckjaz chuckjaz merged commit 4f7d62a into angular:master Mar 15, 2017
SamVerschueren pushed a commit to SamVerschueren/angular that referenced this pull request Mar 18, 2017
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(platform-server): @HostListener('window') doesn't work
8 participants