Bug 146047

Summary: REGRESSION(182495): Error's "column" property should not be readonly
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: JavaScriptCoreAssignee: 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   

Description Joseph Pecoraro 2015-06-16 22:00:57 PDT
* 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
Comment 1 Radar WebKit Bug Importer 2015-06-16 22:01:30 PDT
<rdar://problem/21415612>
Comment 2 Joseph Pecoraro 2015-06-16 22:05:57 PDT
Bisected back to: <http://trac.webkit.org/changeset/182495>
Comment 3 Joseph Pecoraro 2015-06-16 22:08:00 PDT
> > 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);
Comment 4 Joseph Pecoraro 2015-06-16 22:33:45 PDT
(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}
Comment 5 Joseph Pecoraro 2016-08-19 19:01:03 PDT
Ended up addressing this in bug 160984.

*** This bug has been marked as a duplicate of bug 160984 ***