[BUGFIX beta] contextualElements, clean omitted start tags #5571
Conversation
| if (contextualElement && contextualElement.nodeType === 1) { | ||
| contextualElements.unshift(contextualElement); | ||
| } else { | ||
| contextualElements.unshift(contextualElements[0]); |
There was a problem hiding this comment.
These are shifted off on line 110 where it seems foolish to perform the same checks again. Instead, just push the previous contextualElement onto the stack again. Open to some cleaner ideas.
mixonic
force-pushed
the
contextual-element-stack
branch
from
b63df43
to
610c130
Compare
mixonic
force-pushed
the
contextual-element-stack
branch
7 times, most recently
from
95dd242
to
c24b9ed
Compare
|
Aiight, this stack version works, but it does not properly use the morph's existing ability to keep track of contextualElements. Hence I am repeating a bit of work that is unnecessary. Re-working after review with @krisselden |
mixonic
force-pushed
the
contextual-element-stack
branch
from
c24b9ed
to
3583dec
Compare
mixonic
changed the title
mixonic
force-pushed
the
contextual-element-stack
branch
2 times, most recently
from
de11a3a
to
1144b7a
Compare
|
@krisselden ready for another review. Much cleaner. What should have been done from the get-go. Feelin' good. |
|
Changing the RenderBuffer constructor and adding an assert is not backwards compatible. Why not just default to this.dom.document.body if no contextual element is not specified for now? |
|
@mmun I'll change the assert to a deprecation warning, and add a fallback to document.body, sg? I'd like to raise some kind of notice. Anywhere the we fall back to a default context is a chance not only for an end-user to stumble and get bad data back, but also for us to introduce bugs internally. I'll open a PR on HTMLBars for a current context accessor or something. leaking Gracias. |
mixonic
force-pushed
the
contextual-element-stack
branch
from
1144b7a
to
3a91751
Compare
|
updated as described. |
mixonic
force-pushed
the
contextual-element-stack
branch
5 times, most recently
from
5b37fef
to
a55b572
Compare
|
@mixonic does this need review from someone? |
mixonic
force-pushed
the
contextual-element-stack
branch
from
a55b572
to
665bdea
Compare
|
I would like to get a 👍 from @krisseldon on the On 12 Sep 2014, at 17:41, Peter Wagenet wrote:
|
During a renderTree call, look to the morphs for information about the contextualElement. Pass this to the Ember renderer, which in turn passes it to the buffer for use when generating an element. Here I've changed the morph creation points to explicitly pass a contextualElement. This avoids the possibility of createMorph's fallback to document.body happening. Additionally, strip omitted start tags even if the child node causing their generation is after a script tag. See the long comment in `render_buffer`.
mixonic
force-pushed
the
contextual-element-stack
branch
from
665bdea
to
9cb03e8
Compare
|
Closing in favor of #5609 |
During a renderTree call, look to the morphs for information about the contextualElement. Pass this to the Ember renderer, which in turn passes it to the buffer for use when generating an element.
Here I've changed the morph creation points to explicitly pass a contextualElement. This avoids the possibility of createMorph's fallback to document.body happening.
Additionally, strip omitted start tags even if the child node causing their generation is after a script tag. See the long comment in
render_buffer.Fixes #5476