This repository was archived by the owner on Apr 12, 2024. It is now read-only.
Repository navigation
Conversation
1 of 3 tasks
Contributor
|
Did you find any FF bug to reference? |
Member
Author
|
No, I couldn't find anything 😞 |
Narretz
suggested changes
Jul 3, 2018
Narretz
left a comment
Contributor
There was a problem hiding this comment.
LGTM, except that this is a fix instead of a refactor.
DOM nodes passed to `compilationGenerator()` will eventually be wrapped in `jqLite`, when the compilation actually happens. In Firefox 60+, there seems to be a `DocumentFragment`-related bug that sometimes causes the `childNodes` to be empty at the time the compilation happens. This commit works around this bug by eagerly wrapping `childNodes` in `jqLite`. NOTE: The wrapped nodes have references to their `DocumentFragment` container. This is "by design", since we want to be able to traverse the nodes via `nextSibling` (in order to correctly handle multi-element directives). Once the nodes are compiled, they will be either moved to a new container element or the `jqLite` wrapper is release making them eligible for garbage collection. In both cases, the original `DocumentFragment` container should be eligible for garbage collection too. Fixes angular#16607
gkalpak
force-pushed
the
fix-compile-ff-DocumentFragment-bug
branch
from
July 4, 2018 11:29
add51b2 to
0a943b8
Compare
DocumentFragment bugDocumentFragment bug
Member
Author
|
Updated the commit message. |
Narretz
approved these changes
Jul 4, 2018
Contributor
|
Just thought about this more ... should there be a |
Member
Author
|
I thought about this. I decided not to include a comment, since we are really not doing something special (in the sense that |
Contributor
|
Ok, let's jsut merge as is |
gkalpak
added a commit
that referenced
this pull request
Jul 9, 2018
DOM nodes passed to `compilationGenerator()` will eventually be wrapped in `jqLite`, when the compilation actually happens. In Firefox 60+, there seems to be a `DocumentFragment`-related bug that sometimes causes the `childNodes` to be empty at the time the compilation happens. This commit works around this bug by eagerly wrapping `childNodes` in `jqLite`. NOTE: The wrapped nodes have references to their `DocumentFragment` container. This is "by design", since we want to be able to traverse the nodes via `nextSibling` (in order to correctly handle multi-element directives). Once the nodes are compiled, they will be either moved to a new container element or the `jqLite` wrapper is release making them eligible for garbage collection. In both cases, the original `DocumentFragment` container should be eligible for garbage collection too. Fixes #16607 Closes #16615
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Browser bug work-around.
What is the current behavior? (You can also link to an open issue here)
DOM nodes passed to
compilationGenerator()will eventually be wrapped injqLite, when the compilation actually happens. In Firefox 60+, there seems to be aDocumentFragment-related bug that sometimes causes thechildNodesto be empty at the time the compilation happens.What is the new behavior (if this is a feature change)?
This commit works around this bug by eagerly wrapping
childNodesinjqLite.Does this PR introduce a breaking change?
No.
Please check if the PR fulfills these requirements
Fix/Feature: Docs have been added/updatedFix/Feature: Tests have been added; existing tests passOther information:
Fixes #16607.
NOTE:
The wrapped nodes have references to their
DocumentFragmentcontainer. This is "by design", since we want to be able to traverse the nodes vianextSibling(in order to correctly handle multi-element directives).Once the nodes are compiled, they will be either moved to a new container element or the
jqLitewrapper is release making them eligible for garbage collection. In both cases, the originalDocumentFragmentcontainer should be eligible for garbage collection too.