Skip to content
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

IE8 'movesWhitespace' issue #3650

Closed
gavro opened this issue Oct 25, 2013 · 4 comments
Closed

IE8 'movesWhitespace' issue #3650

gavro opened this issue Oct 25, 2013 · 4 comments

Comments

@gavro
Copy link

gavro commented Oct 25, 2013

I've been having problems with your movesWhiteSpace check & fix, and it seems it might be just your findChildByID function, which is not returning anything.

You can find a reproduction of the bug here: http://jsfiddle.net/3EwXR/3/

To reproduce this with your own code, just try adding a conditional like this:

<span class="{{#if active_index}} active {{/if}}">TEST1</span>

You NEED the spaces between }} [class_name] {{
--> this conditional is simply not working:

<span class="{{#if active_index}}active{{/if}}">TEST1</span>

(Could someone maybe explain why you need to add spaces here?)

So you've got the following code (r18534):

  // If we have to do any whitespace adjustments do them now
  if (matches.length > 0) {
    var len = matches.length, idx;
    for (idx=0; idx<len; idx++) {
      var script = findChildById(element, matches[idx][0]),
          node = document.createTextNode(matches[idx][1]);
      script.parentNode.insertBefore(node, script);
    }
  }

--> your comment @ findChildByID: // Use this to find children by ID instead of using jQuery

If you change the above code to this (using jQuery find) it seems to be working:

  // If we have to do any whitespace adjustments do them now
  if (matches.length > 0) {
    var len = matches.length, idx;
    for (idx=0; idx<len; idx++) {
      var script = Ember.$(element).find('#'+matches[idx][0]),
          node = document.createTextNode(matches[idx][1]);
      script.insertBefore(node, script);
    }
  }

Whan applying this new code: the page get's rendered. But the active state is NOT applied. Your whiteSpaceFix removes the trailing space, so you get:

<span class="{{#if active_index}} active{{/if}}">TEST1</span>

...which again, is not working.

To get this completely working in IE8, I need to disable the whitespace fix completely:

  /*if (movesWhitespace) {
    // Right now we only check for script tags with ids with the
    // goal of targeting morphs.
    html = html.replace(/(\s+)(<script id='([^']+)')/g, function(match, spaces, tag, id) {
      matches.push([id, spaces]);
      return tag;
    });
  }*/

Any thoughts on this?

@alexspeller
Copy link
Contributor

You should be using bindAttr, not {{if}} statements for attributes.

@gavro
Copy link
Author

gavro commented Oct 26, 2013

Hmm, check, that solves it in this case. Thanks!
But isn't the removal of the trailing space still something that might break some sites/logic @ IE8?

Offtopic, it a shame (imho):

<span {{bind-attr class="active_index:active:otherstatus :anotherClass :extra"}}>TEST1</span>

is not really intiutive (for frontenders, using ember.js for protoyping). IF/else logic with ":" seperators. Prepending static classes with ":"... I do get the need for bind-attr though, as Handlebars is not context aware (prevent script-tag output)

See here: http://jsfiddle.net/gavro/3EwXR/4/

@rwjblue
Copy link
Member

rwjblue commented Oct 26, 2013

The basic reason that you cannot use if/else within tags (as you originally had) is that Handlebars uses metamorph tags to surround dynamic segments and know what needs to be changed/updated. bind-attr uses attributes instead of tags so that you can use it from within another tag.

There is definitely a plan to remove this limitation (https://github.com/tildeio/htmlbars), but there is no timeline that I am aware of.

@stefanpenner
Copy link
Member

@gavro i believe this is a handlebars thing: handlebars-lang/handlebars.js#336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants