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

Mustache: DOM exception when applying certain block patterns #243

Closed
bmomberger-reciprocity opened this Issue Jan 19, 2013 · 12 comments

Comments

Projects
None yet
6 participants
@bmomberger-reciprocity
Contributor

bmomberger-reciprocity commented Jan 19, 2013

http://jsfiddle.net/B8kWR/1/

Similar to #241 but outside of attributes, when using can.Observes to live-bind, this pattern will cause an error in the escape case in can.Mustache.txt (adding a newly generated text node). This is on Chrome 24; other browsers have not been tested.

Workaround: for the case of "show a, or b if a does not exist" try these helpers from github/recipocity/grc:

/*
  Usage:  {{firstexist possibly_null possibly_null_as_well 'fallback' }}
              {{firstnonempty possibly_empty possibly_empty_as_well 'fallback' }}
*/
can.each(["firstexist", "firstnonempty"], function(fname) {
  Mustache.registerHelper(fname, function() {
    var args = can.makeArray(arguments).slice(0, arguments.length - 1);
    for(var i = 0; i < args.length; i++) {
      if(typeof v === "function") v = v.call(this);
      var v = args[i];
      if(v != null && (fname === "firstexist" || !!(v.toString().trim()))) return v;
    }
    return "";
  });
});
@pmccartney

This comment has been minimized.

Show comment
Hide comment
@pmccartney

pmccartney Jan 22, 2013

Is this referring to "NOT_FOUND_ERR: DOM Exception 8" ? I was about to post a bug about this. Here is what I have seen; Given a model with attr 'name' and a proto function isLoggedIn(), handing it to the following mustache file, this will cause the exception.

{{#isLoggedIn}}{{name}}{{/isLoggedIn}}

Can.js : line 5872

Same error in FF: "Not Found Error: Node was not found"

pmccartney commented Jan 22, 2013

Is this referring to "NOT_FOUND_ERR: DOM Exception 8" ? I was about to post a bug about this. Here is what I have seen; Given a model with attr 'name' and a proto function isLoggedIn(), handing it to the following mustache file, this will cause the exception.

{{#isLoggedIn}}{{name}}{{/isLoggedIn}}

Can.js : line 5872

Same error in FF: "Not Found Error: Node was not found"

@bmomberger-reciprocity

This comment has been minimized.

Show comment
Hide comment
@bmomberger-reciprocity

bmomberger-reciprocity Jan 22, 2013

Contributor

That is indeed the issue I am seeing. As far as I can tell, it's not a null-reference type of issue; it may be that we are causing CanJS to try to insert a text node in a parent while providing a positioning reference that is not a child of the parent. I have not yet investigated it further than this.

Contributor

bmomberger-reciprocity commented Jan 22, 2013

That is indeed the issue I am seeing. As far as I can tell, it's not a null-reference type of issue; it may be that we are causing CanJS to try to insert a text node in a parent while providing a positioning reference that is not a child of the parent. I have not yet investigated it further than this.

@jbmillini

This comment has been minimized.

Show comment
Hide comment
@jbmillini

jbmillini Jan 22, 2013

Ignore this comment. Apologies about the spam. Testing our community page.

jbmillini commented Jan 22, 2013

Ignore this comment. Apologies about the spam. Testing our community page.

@pmccartney

This comment has been minimized.

Show comment
Hide comment
@pmccartney

pmccartney Jan 23, 2013

This is actually a pretty major issue as you can't even do simple bound lists with mustache. Even #each doesn't work.
{{#users}}{{name}}{{/users}} or
{{#each users}}{{name}}{{/each}} fails

Who decides when this should be labeled a bug?

pmccartney commented Jan 23, 2013

This is actually a pretty major issue as you can't even do simple bound lists with mustache. Even #each doesn't work.
{{#users}}{{name}}{{/users}} or
{{#each users}}{{name}}{{/each}} fails

Who decides when this should be labeled a bug?

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jan 23, 2013

Contributor

This is the same issue as #153 right?

Contributor

daffl commented Jan 23, 2013

This is the same issue as #153 right?

@ghost ghost assigned andykant Jan 23, 2013

@pmccartney

This comment has been minimized.

Show comment
Hide comment
@pmccartney

pmccartney Jan 23, 2013

Yes. Sorry, it is. At least for the cases I am running into.

pmccartney commented Jan 23, 2013

Yes. Sorry, it is. At least for the cases I am running into.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jan 23, 2013

Contributor

Ok makes sense. Does the workaround mentioned there work for you for? We'll try our best making this fix a priority for Mustache in the 1.1.4 release.

Contributor

daffl commented Jan 23, 2013

Ok makes sense. Does the workaround mentioned there work for you for? We'll try our best making this fix a priority for Mustache in the 1.1.4 release.

@pmccartney

This comment has been minimized.

Show comment
Hide comment
@pmccartney

pmccartney Jan 24, 2013

Works great! All the work that has been put into this is incredible.

pmccartney commented Jan 24, 2013

Works great! All the work that has been put into this is incredible.

@bmomberger-reciprocity

This comment has been minimized.

Show comment
Hide comment
@bmomberger-reciprocity

bmomberger-reciprocity Feb 7, 2013

Contributor

With 1.1.4, using an {{#if}} block no longer throws errors, but also does not display text. My comments in #241 seem to apply here as well.

updated fiddle for 1.1.4: http://jsfiddle.net/B8kWR/2/

Contributor

bmomberger-reciprocity commented Feb 7, 2013

With 1.1.4, using an {{#if}} block no longer throws errors, but also does not display text. My comments in #241 seem to apply here as well.

updated fiddle for 1.1.4: http://jsfiddle.net/B8kWR/2/

@andykant

This comment has been minimized.

Show comment
Hide comment
@andykant

andykant Mar 20, 2013

Contributor

This is fixed with the fixes for #153.

Updated fiddle with the fix copy+pasted: http://jsfiddle.net/B8kWR/8/
I modified the test templates slightly because {^if text} isn't a valid helper call (should be {#unless text} or {^text}).

Contributor

andykant commented Mar 20, 2013

This is fixed with the fixes for #153.

Updated fiddle with the fix copy+pasted: http://jsfiddle.net/B8kWR/8/
I modified the test templates slightly because {^if text} isn't a valid helper call (should be {#unless text} or {^text}).

@andykant andykant closed this Mar 20, 2013

@airhadoken

This comment has been minimized.

Show comment
Hide comment
@airhadoken

airhadoken Mar 20, 2013

Contributor

Eep. I also used {{^if ...}} in the test for #292 (it passed, though). Would you mind boyscouting that if you are working near it?

https://github.com/bitovi/canjs/blob/master/view/mustache/test/mustache_test.js#L1342

Thanks.

Contributor

airhadoken commented Mar 20, 2013

Eep. I also used {{^if ...}} in the test for #292 (it passed, though). Would you mind boyscouting that if you are working near it?

https://github.com/bitovi/canjs/blob/master/view/mustache/test/mustache_test.js#L1342

Thanks.

@andykant

This comment has been minimized.

Show comment
Hide comment
@andykant

andykant Mar 20, 2013

Contributor

It looks like that does work now. That must have been something that changed between the version of Mustache in that fiddle and latest.

Contributor

andykant commented Mar 20, 2013

It looks like that does work now. That must have been something that changed between the version of Mustache in that fiddle and latest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment