Android 4.0 issue about native-promise-only

[smikes]

Thanks for the bug report.

[smikes]

Could you try out the candidate fix in 306216f and let me know if it works? I don't have an appropriate device to test with.

[ironsidevsquincy]

Will do. Clocking off time here in England, I'll try tomorrow morning

[smikes]

OK, thanks!

On Wed, Nov 5, 2014 at 10:55 AM, Darren Hurley [email protected]
wrote:

Will do. Clocking off time here in England, I'll try tomorrow morning

—
Reply to this email directly or view it on GitHub
#38 (comment)
.

[ironsidevsquincy]

Hmmm, doesn't quite work. Looks like it only fails specifically when setting the prototype on a function, i.e.

var func  = function () {},
    proto = {};
builtInProp(func, 'prototype', proto, false);

alert(func.prototype === proto);

alerts false, as expected. If func = {}, alerts true

[smikes]

Thanks - will fix it

On Thu, Nov 6, 2014 at 4:15 AM, Darren Hurley [email protected]
wrote:

Hmmm, doesn't quite work. Looks like it only fails specifically when setting the prototype on a function, i.e.

var func  = function () {},
    proto = {};
builtInProp(func, 'prototype', proto, false);
alert(func.prototype === proto);

alerts false, as expected. If func = {}, alerts true

Reply to this email directly or view it on GitHub:
#38 (comment)

[smikes]

I made x be a function () {} -- when you have a chance, let me know if the new fix works.

Thanks!

[smikes]

Unfortunately my test was incorrect - it should check for

var y = function() {} , x = {};

Object.defineProperty(y, "prototype", { value: x });
if (y.prototype === x) throw;

Can you please verify, again, with the new corrected code?

[getify]

(FYI: I updated my suggested gist with this correction, too: https://gist.github.com/getify/906aadd914a675ea4a57)

[deleteme]

@smikes Confirming that 67d8223 fixes it. It look me a little longer to test this because a build step is required. For what it's worth, npm build doesn't work, but node build does.

[smikes]

Thanks.

Yes, npm build would run node-gyp if there was a bindings.gyp, while npm run-script build is necessary to run the 'build' script defined in package.json

[mgol]

jQuery is now using NPO to test compliance of its Deferreds against the Promises/A+ tests and this issue is causing jQuery.when tests to fail. See http://swarm.jquery.org/result/99082

What else is needed to get #38 merged?

[getify]

Sorry that this got hung up for so long on #41. We were discussing possible file reorg and tests and such, and I think we missed the forest for the trees. So I've just landed a fix only for the Android thing (hopefully!).

[mgol]

@getify Thanks! It turns out jQuery test errors are caused by another issue, see #43.

[getify]

Actually, I didn't fix this issue correctly after all. :(

[getify]

Man, this android4 browser behavior is insane. If you use Object.defineProperty, it actually does change the descriptor, but not the underlying function value. Then, even better, subsequently if you try to set it with normal = assignment, THAT assignment silently fails. = assignment only works if you haven't previously done the Object.defineProperty.

[getify]

For posterity sake, the observation I made was that the builtInProp(..) usage before was actually setting the same sort of descriptor (writable:true, enumerable:false, configurable:false) as is already true of function prototypes (per spec)... so the normal = assignment is shorter, accomplishes the exact same outcome, and most importantly doesn't trigger this android4 stupidity.

Ironic/unfortunate that the OP suggested exactly this, and it took me all this time and wrapping around the issue to come to the same conclusion. :/