Bug 146047
| Summary: | REGRESSION(182495): Error's "column" property should not be readonly | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> |
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED DUPLICATE | ||
| Severity: | Normal | CC: | ggaren, joepeck, mark.lam, mmirman, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Joseph Pecoraro
* SUMMARY
Error's "column" property should not be readonly. This used to work in Safari 8, but now fails in ToT WebKit.
* TEST
<script>
"use strict";
var error = new Error('My Error Message');
error.lineNumber = 1;
error.column = 2; // Should not trigger a TypeError.
console.log("SUCCESS", error);
</script>
* STEPS TO REPRODUCE
1. Open attached test case
=> get an error attempting to set error.column, used to work
=> [Error] TypeError: Attempted to assign to readonly property.
* NOTES
- Firefox and Chrome do not throw an error
- This is done in the Esprima JS Library, so there may also be other libraries that do this and expect it to work
* DESCRIPTORS
> Object.getOwnPropertyDescriptor(error, "column")
< {value: 22, writable: false, enumerable: true, configurable: false} = $2
> Object.getOwnPropertyDescriptor(error, "lineNumber")
< {value: 1, writable: true, enumerable: true, configurable: true} = $3
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/21415612>
Joseph Pecoraro
Bisected back to: <http://trac.webkit.org/changeset/182495>
Joseph Pecoraro
> > Object.getOwnPropertyDescriptor(error, "lineNumber")
> < {value: 1, writable: true, enumerable: true, configurable: true} = $3
Ahh, this is misleading since no such property already existed, this was just added by me.
I think the relevant part of the original change was:
+ obj->putDirect(vm, vm.propertyNames->line, jsNumber(line), ReadOnly | DontDelete);
+ obj->putDirect(vm, vm.propertyNames->column, jsNumber(column), ReadOnly | DontDelete);
Joseph Pecoraro
(In reply to comment #3)
> > > Object.getOwnPropertyDescriptor(error, "lineNumber")
> > < {value: 1, writable: true, enumerable: true, configurable: true} = $3
>
> Ahh, this is misleading since no such property already existed, this was
> just added by me.
>
> I think the relevant part of the original change was:
>
> + obj->putDirect(vm, vm.propertyNames->line, jsNumber(line), ReadOnly
> | DontDelete);
> + obj->putDirect(vm, vm.propertyNames->column, jsNumber(column),
> ReadOnly | DontDelete);
Actually the original code had something very similar. So I don't actually see how this was writable before... but there is certainly some visible change here.
Other browsers have different property names and descriptor attributes for the error properties. There is no standardization at all with Errors. That said, making the property writable seems reasonable.
Firefox:
js> Object.getOwnPropertyNames(new Error)
Array [ "fileName", "lineNumber", "columnNumber" ]
js> Object.getOwnPropertyDescriptor(new Error, "lineNumber")
Object { value: 1, writable: true, enumerable: false, configurable: true }
Chrome:
js> Object.getOwnPropertyNames(new Error)
["stack"]
js> Object.getOwnPropertyDescriptor(new Error, "stack")
Object {enumerable: false, configurable: true}
Joseph Pecoraro
Ended up addressing this in bug 160984.
*** This bug has been marked as a duplicate of bug 160984 ***