Binding type attribute on a button element fails in PhantomJS

[simonihmig]

Some unit tests testing for correct HTML attributes, bound via attributeBindings on a <button> component, started to fail suddenly using Ember 1.11 in https://github.com/kaliber5/ember-bootstrap.

I created a separate repository to isolate that possible bug: https://github.com/simonihmig/ember-bind-attributes-test
There you will find a button component, with its typeset to submit via attributeBindings, tested with a unit test. And a simple <button {{bind-attr type=buttonType}}> HTML element (using old style attribute bindings for 1.10 compatibility), tested with an acceptance test.

Running ember try:testall will run both tests, using ember-try with 1.10, 1.11, beta and canary, in PhantomJS as well as Chrome.

The results are:

• everything works as expected in Chrome
• the button component (attributeBindings) has no type attribute in PhantomJS, for Ember 1.11, beta and canary, so tests fail. The test for Ember 1.10 succeeds though! Something probably must have changed here, maybe related to http://emberjs.com/blog/2015/03/27/ember-1-11-0-released.html#toc_bound-attribute-syntax
• the button HTML element ({{bind-attr}}) fails in PhantomJS, for all Ember versions!!? Strange...

I also added tests for some other button properties, like role="button", but these seem to be ok. So there seems to be some spacial case here for the type attribute in PhantomJS. One could blame PhantomJS solely here, but that would not explain why the button component tests starts failing only beginning with Ember 1.11.

Although normal Browsers does not seem to be affected, it would still be very nice to have this fixed, for CI tests where PhantomJS plays its role...

[rwjblue]

We changed from using jQuery to set attributeBindings to using direct DOM methods (via our own internal DOMHelper) in 1.11. So in 1.10 (which works) we are using $element.attr('type', 'submit'), and in 1.11+ we are using (something like) element.type = 'submit' (which blows up apparently in Phantom).

[simonihmig]

Ok, thanks for clarification.
I did a quick check how PhantomJS behaves:

var el = document.createElement('button');

el.setAttribute('type','submit');
console.log(el.getAttribute('type')); // output is: submit

var el2 = document.createElement('button');

el2.type = 'submit';
console.log(el2.getAttribute('type')); // output is: null

I am not so sure how the exact DOM specification is, can this be regarded a bug in Phantom?

If so, I can file a bug report for PhantomJS, but any chance to have a workaround in ember in the meantime?

[rwjblue]

We are not using setAttribute, we are setting this as a property like:

var el = document.createElement('button');
el['type'] = 'submit';

Which apparently throws an error in PhantomJS.

[simonihmig]

Yes, I did understand that. That's what I tested with el2 in the example above.

So back to my other question: can we clearly blame PhantomJS here for being not standards compliant, or aren't we supposed to use setAttribute? And if it is PhantomJS' fault, is there still a chance for a workaround in ember?

[rwjblue]

That's what I tested with el2 in the example above.

HA! Indeed you did, sorry about that!

can we clearly blame PhantomJS here

Yes, I think we can.

is there still a chance for a workaround in ember?

It is possible for us to work around (we did a similar thing in #10690 for <input> elements).

@mixonic - Thoughts?

[mixonic]

@rwjblue it would fix this and the other issue if we just whitelisted input type to setAttribute, right?

If so, I'm all for it.

[rwjblue]

@mixonic I'll confirm, but yes I think that is correct.

[Robdel12]

Experiencing something similar in Emberx-select. The form attr is no long binding to the select:

https://github.com/thefrontside/emberx-select/blob/update/ember-cli-0.2.4/tests/dummy/app/templates/single.hbs#L2

Failing build: https://travis-ci.org/thefrontside/emberx-select/jobs/62926462

Any suggestions?

[stefanpenner]

@rwjblue

  % ./bin/phantomjs                                                                                                             !13236
phantomjs> var el = document.createElement('button');
undefined
phantomjs> el['type'] = 'submit';
"submit"
phantomjs> el.type
"submit"

  % ./bin/phantomjs -v                                                                                                          !13236
1.9.8

[simonihmig]

@stefanpenner confirmed! However getAttribute returns null when setting type without setAttribute:

phantomjs> var el = document.createElement('button');
undefined
phantomjs> el['type'] = 'submit';
"submit"
phantomjs> el.type
"submit"
phantomjs> el.getAttribute('type')
null
phantomjs> el.setAttribute('type', 'submit')
undefined
phantomjs> el.getAttribute('type')
"submit"

[rwjblue]

This is because the type of submit is not reflected as an attribute (because type is an enumerated attribute it can only have specific known values reflected as an attribute).

You can fix the tests in your repo by not using $el.attr('type') but instead use $el.prop('type').

Further reading (credit to @mixonic for all of the correct details, any mistakes are mine 😄 ):

• https://html.spec.whatwg.org/multipage/forms.html#the-input-element
• http://dev.w3.org/html5/spec-preview/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes

[stefanpenner]

@stefanpenner confirmed! However getAttribute returns null when setting type without setAttribute:

good point

[stefanpenner]

more thoughts: tildeio/htmlbars#353 (comment)

[simonihmig]

Another one:

phantomjs> var el = document.createElement('button');
undefined
phantomjs> el['type'] = 'button';
"button"
phantomjs> el.type
"submit"
phantomjs> el.setAttribute('type', 'button');
undefined
phantomjs> el.type
"button"

It's getting funny! :)

@rwjblue Thanks for that $el.prop('type') hint. It fixes one test, where I expect the button to be of type "submit" (which it is by default anyway). However a test with a button-type button still fails this way. It seems even the property is not changed unless using setAttribute. See example above.

[stefanpenner]

If we could somehow get Travis to update Phantom to 2.0 we can avoid all of this headache

i think some IE have this issue as-well.

[Robdel12]

@bcardarella You can tell travis to use phantom 2:

https://github.com/truenorth/ember-cli-materialize/blob/master/.travis.yml#L20

Credit to @truenorth

[jayphelps]

Just chiming in to confirm this is non-compliance of the spec in phantomjs et al. Whack a mole fixes 👍

Assuming I'm not drunk, here's a PR to fix tildeio/htmlbars#355