search

tildeio / htmlbars

A variant of Handlebars that emits DOM and allows you to write helpers that manipulate live DOM nodes
MIT License
1.61k stars 193 forks source link

Handle null/undefined url in nodeProtocolForURL #455

Closed ronco closed 8 years ago

ronco commented 8 years ago

The node version of protocolForURL was throwing an exception if the url is undefined here. This is invoked in sanitizeAttributeValue. This was calling issues for bound attributes with no value in FastBoot environments.

This PR returns a sensible default (:) from nodeProtocolForURL when the url argument is a non-string.

I'm not sure how to test this though. I'd like to get a test in place if possible before merging.

/cc @tomdale

tomdale commented 8 years ago

I think the best bet is to add a failing test to Ember.js. We can see that PR fail, then merge this fix and cut a new release, then see the PR pass and then merge the bumped version into Ember.

tomdale commented 8 years ago

Fix looks good btw.

rwjblue commented 8 years ago

Will also need to be fixed in tildeio/glimmer

rwjblue commented 8 years ago

Still needs to be done in tildeio/glimmer, if we forget this it will regress...

ronco commented 8 years ago

@rwjblue yes, @tomdale is going to follow up with @chancancode to get some direction there.

tomdale commented 8 years ago

According to @chancancode the work has not yet started on the Node.js-compatible DOMHelper, what's the best way to track this?