-
Notifications
You must be signed in to change notification settings - Fork 27.3k
fix($compile): work around Firefox DocumentFragment bug
#16615
fix($compile): work around Firefox DocumentFragment bug
#16615
Conversation
|
Did you find any FF bug to reference? |
|
No, I couldn't find anything 😞 |
Narretz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
add51b2 to
0a943b8
Compare
DocumentFragment bugDocumentFragment bug
|
Updated the commit message. |
|
Just thought about this more ... should there be a |
|
I thought about this. I decided not to include a comment, since we are really not doing something special (in the sense that |
|
Ok, let's jsut merge as is |
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
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.