1000254 - options() always returns the empty string

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Jason Orendorff [:jorendorff]]

That this bug went unnoticed for 6 months or so demonstrates that optionsClear() and optionsPush() in js/src/tests/shell.js are not being seriously used...

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: Part 1 - Fix options() return value.

The JS_ReportError changes are just en passant simplification.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: Part 2 - Remove some unused options-related junk from tests/{shell,browser}.js

Removes ineffectual jit() calls from a bunch of tests. Why not.

Not touched in this patch: a handful of tests disabled 4 years ago in bug 540512. :-P

Comment 4

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/27d99e7d0820

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/27d99e7d0820

Comment 6

[Jeff Walden [:Waldo]]

Comment on attachment 8411289 [details] [diff] [review]
Part 2 - Remove some unused options-related junk from tests/{shell,browser}.js

Review of attachment 8411289 [details] [diff] [review]:
-----------------------------------------------------------------

Again, sorry for the delay...

::: js/src/tests/browser.js
@@ +125,5 @@
>    if (typeof jstestsRestoreFunction === "function") {
>      jstestsRestoreFunction();
>    }
>  
> +  optionsReset();

Doesn't your change to shell.js remove this function, so requiring this call to be removed?

@@ +188,5 @@
>    // return value of options() is a comma delimited list
>    // of the previously set values
> +  var names = [for (name of options.names)
> +                 if (SpecialPowers.Cu[name])
> +                   name];

I believe our harness still works for testing in other browsers, and an array comprehension here would break that.  Please do this like |options.names.filter(function(name) { return SpecialPowers.Cu[name]; })|.

@@ +407,5 @@
>      print(PASSED);
>      jstestsTestPassesUnlessItThrows = false;
>    }
>  
> +  optionsReset();

Again, didn't you kill this off?

Comment 7

[Ben McCann]

Hey, just wanted to check in on this really quickly. This'd be a nice one for us and it looks like there's already a patch. Much chance of it getting checked in? Thanks!

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:jorendorff, maybe it's time to close this bug?