User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release) Build ID: 20130816030205 Steps to reproduce: Occasionally, deleting a localStorage property via a delete expression (rather than via removeItem) results in an error. For example, sometimes the expression `delete localStorage["iD_http://osm.dev_lock"]` results in the error 'TypeError: property "iD_http://osm.dev_lock" is non-configurable and can't be deleted', while `localStorage.removeItem("iD_http://osm.dev_lock")` always works fine.

Thanks for reporting this John. Could you provide a minimal testcase or an URL to better understand and reproduce this?

Unfortunately I was not able to minimize this, but I can tell you that it occurred in the iD editor for OpenStreetMap. The following issues were caused by it, and switching from delete to removeItem solved the issue: https://github.com/systemed/iD/issues/1691 https://github.com/systemed/iD/issues/1692

I have exactly same problem, and also failed at attempt to produce minimal test case. It's valid issue.

Is there a non-minimal but _reliable_ testcase?

Boris, try http://ldz.eregistrations.org/ and open console, you'll see "TypeError: property "X" is non-configurable and can't be deleted" errors, and blank page cause of that. This page worked well on Firefox about 2 weeks ago.

That indeed reproduces reliably, thanks

One-day m-c range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3d40d270c031&tochange=c2b375f3a909

Note, of course, that my range is different from "2 weeks ago".. On the site in comment 5, what I see is that we're in a strict-mode script (specifically http://ldz.eregistrations.org/public.js which has code like this: if (userId) localStorage._authenticated = userId; else delete localStorage._authenticated; But if localStorage._authenticated is not actually set (which in my case of course it's not), then what will this do? Per spec, the object has a named deleter, and this deleter has an identifier and the operation returns void. So we should land in http://dev.w3.org/2006/webapi/WebIDL/#delete step 2.5 and the delete should never throw. OK, so what does our implementation do? This class is still using XPConnect bindings, so we'll call XPC_WN_Helper_DelProperty. In bug 858677 this function picked up a "JSBool *succeeded" out param that it never sets (red flags should be going off now). And then the calling code in Interpret() is: 2190 bool succeeded; 2191 if (!JSObject::deleteProperty(cx, obj, name, &succeeded)) 2192 goto error; 2193 if (!succeeded && script->strict) { 2194 obj->reportNotConfigurable(cx, NameToId(name)); So hey, we're testing random memory. On the site in comment 5 we seem to get lucky in that the memory is reliably 0 (or at least became so in the regression range in comment 7).

So should XPC_WN_Helper_DelProperty just *succeeded = true up front and be done with it? That certainly fixes the site in comment 5, and I bet it'll fix OpenStreetMap too.

(In reply to Boris Zbarsky [:bz] from comment #9) > So should XPC_WN_Helper_DelProperty just *succeeded = true up front and be > done with it? That certainly fixes the site in comment 5, and I bet it'll > fix OpenStreetMap too. Yeah I think so. Apparently the SH hook never got a similar update, and it probably isn't worth doing at this point. r=me on |*succeeded = true|.

Comment on attachment 811993 [details] [diff] [review] Make sure to initialize the "succeeded" outparam of XPC_WN_Helper_DelProperty. [Approval Request Comment] Bug caused by (feature/regressing bug #): 858677 User impact if declined: Some sites (e.g. OpenStreetMap) randomly fail to work. Other sites reliably fail to work. Testing completed (on m-c, etc.): Makes the one reliable testcase we have that shows the bug work right. Risk to taking this patch (and alternatives if risky): I believe this is extremely low risk. String or IDL/UUID changes made by this patch: None

I'm told this'll become WebIDL soon, and in there this delete always succeeds, so this seems a totally fine stopgap til WebIDL. Nice to see this so simply fixed, for branches' sake.

Great thanks! Is this scheduled to land in stable FF soon?

I've requested approval to land it on Aurora 26 and Beta 25. If that's granted (which I expect it will be), this will ship in Firefox 25, on Oct 29 or so. The only way to get it out there faster is to do a special "chemspill" release. It's not quite clear to me whether that's worth it; it would shave maybe 2 weeks off the time to getting this out. If you need an urgent workaround, removeItem should work, or putting the script in non-strict mode for now, right? :(

Boris, thanks for info. Application is still in dev phase, and I think we can manage with 2 weeks wait. I just wanted to make sure it'll be addressed for stable channel quite soon. I can hardcode workarounds directly in code, but prefer not to do that.

(In reply to Mariusz Nowak from comment #19) > Boris, thanks for info. Application is still in dev phase, and I think we > can manage with 2 weeks wait. (Note that it'll be 4 weeks, not 2. 2 is what it would be if we chemspilled).

4 weeks also should be fine, if not, then I'll probably somehow manage that on my side. I just wonder if it's indeed just few sites that were affected, as it's quite serious when it happens.

Mariusz, the affected sites need to be using delete on a Storage object in a strict-mode script... and then the stack has to end up such that that particular garbage value is 0, not any other value. So I suspect the number of sites affected is in fact fairly low (if nothing else because of the strict-mode requirement). And even for some of those it may not happen all the time (e.g. OpenStreetMap), because of the "stack must have a 0" requirement. That said, I definitely agree this is a serious bug. That's why I want to backport this to 25 and 26. The bar for doing a chemspill is a lot higher than that, though: it needs to be either a compat issue that breaks a large fraction of sites (by time visited, not number; so e.g. breaking the single site mail.yahoo.com would count) for a significant fraction of our user base or an actively exploited security problem... I'm not quite sure that this bug hits that bar, though I'll let the release folks judge that.

Boris, thanks for clarification.

Comment on attachment 811993 [details] [diff] [review] Make sure to initialize the "succeeded" outparam of XPC_WN_Helper_DelProperty. Given the low risk evaluation here, approving for pre-release branches.

https://hg.mozilla.org/releases/mozilla-aurora/rev/a4b7282df517 https://hg.mozilla.org/releases/mozilla-beta/rev/0c4f0e9b09e9