756214 - test-content-proxy.testStringOverload: failure, timed out

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This test started failing after Irakli landed pull request 428 to fix some issues brought up in the #jetpack channel after bug 743359 landed.

It seems to have only broken debug OSX builds, all Linux builds, and no Windows builds.

error: TEST FAILED: test-content-proxy.testStringOverload (failure)
error: fail: toString can't be called with this being the proxy ("too much recursion" doesn't match /String.prototype.toString called on incompatible Proxy/)
info: Traceback (most recent call last):
  File "resource://e34f02fe-1049-477f-9257-17640e7acfed-at-jetpack/api-utils/tests/test-content-proxy.js", line 45, in onDOMReady
    callback(helper, test);
  File "resource://e34f02fe-1049-477f-9257-17640e7acfed-at-jetpack/api-utils/tests/test-content-proxy.js", line 281, in null
    "toString can't be called with this being the proxy");
  File "resource://e34f02fe-1049-477f-9257-17640e7acfed-at-jetpack/api-utils/lib/unit-test.js", line 125, in assertRaises
    this.assertMatches(errorMessage, predicate, message);
  File "resource://e34f02fe-1049-477f-9257-17640e7acfed-at-jetpack/api-utils/lib/unit-test.js", line 105, in assertMatches
    this.fail(message);
  File "resource://e34f02fe-1049-477f-9257-17640e7acfed-at-jetpack/api-utils/lib/unit-test.js", line 71, in fail
    this.console.trace();

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Correcting the dependencies.

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Alex, any chance you and Eddy can figure out how to fix this test so our proxies work with the new Harmony proxies?

Comment 3

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/af47356cbe41d84017e02df181dad4f0de593087
Merge pull request #444 from Gozala/hotfix/workaround@756214

Bug 756214 - Temporarily disable breaking proxy test a=@gozala

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Alex I have temporarily disabled breaking proxy test to keep tree green while you and Eddy will figure out what should we do with this and future proxy stuff.

Comment 5

[Alexandre Poirot [:ochameau]]

Ok so the following code doesn't work anymore:

var str = "foo";
var proxy = Proxy.create({
  get:function (r,n) {
    return str[n].bind(str);
  }
});
document.querySelectorAll(proxy);

It is returning a NodeList on non-nightly and throw "SyntaxError: An invalid or illegal string was specified" on nightly.

Eddy: Any idea why it throw such exception? Can we make querySelectorAll work again with a JS proxy? By using any JS workaround or patching the platform code.
Otherwise we may have to wait landing this proxy patch before we can get rid of JS proxies.

Comment 6

[Alexandre Poirot [:ochameau]]

eddy: The bug mentioned in comment 5 is now fixed.
Now, we are having a different kind of exception:

var string = "string";
var p = Proxy.create({
  get: function(receiver, name) {
    return string[name];
  }
});
p.toString()
// Used to throw "String.prototype.toString called on incompatible Proxy"
// Now throw "too much recursion"

Isn't it hiding another platform issue? Shouldn't we receive the same exception?

Comment 7

[Alexandre Poirot [:ochameau]]

Assigning to Eddy, as it seems to require some platform fix.

Comment 8

[Tomislav Jovanovic :zombie]

Attachment: Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1413

it seems this got fixed in platform sometime in the last 18 months.

i just updated the test to the new assert module and the Error message wording.

Comment 9

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/7f51ad7b2a78c4e1b16d822938bfb2887713e09f
bug 756214 - update, re-enable testStringOverload

https://github.com/mozilla/addon-sdk/commit/12911bf3216a66aa61b4bf6a07b8d433921808de
Merge pull request #1413 from zombie/756214-test-js-proxy

bug 756214 - update, re-enable testStringOverload r=@gozala