Bug 146047 - REGRESSION(182495): Error's "column" property should not be readonly
Summary: REGRESSION(182495): Error's "column" property should not be readonly
Status: RESOLVED DUPLICATE of bug 160984
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-16 22:00 PDT by Joseph Pecoraro
Modified: 2016-08-19 19:01 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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 ***