1333045 - Location object changed spec violations

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[André Bargull [:anba]]

From https://html.spec.whatwg.org/multipage/browsers.html#the-location-interface:

1. "toString", "valueOf", and "toJSON" are specified as non-enumerable, but are enumerable in Firefox.
2. window.location.valueOf isn't the intrinsic object %ObjProto_valueOf% (`window.location.valueOf === Object.prototype.valueOf` is false, but should be true).
3. window.location.toString isn't the intrinsic object %ObjProto_toString% (`window.location.toString === Object.prototype.toString` is false, but should be true). The spec seems to contradict itself, because later in the spec toString() is described as "Returns the Location object's URL.".

Comment 1

[Anne (:annevk)]

Could you file a bug on the toString() behavior? We should remove that declaration as it appears to be broken. It was meant to mimic https://github.com/heycam/webidl/commit/20a1c3d310f84b103ba0bf429aa4d2a2658f6fa9, but the fact that the Location IDL already contained a [Unforgeable] stringifier overrode that default behavior.

https://www.w3.org/Bugs/Public/show_bug.cgi?id=27361 has the rationale for changing [[Enumerable]].

Comment 2

[André Bargull [:anba]]

(In reply to Anne (:annevk) from comment #1)
> Could you file a bug on the toString() behavior? We should remove that
> declaration as it appears to be broken. It was meant to mimic
> https://github.com/heycam/webidl/commit/
> 20a1c3d310f84b103ba0bf429aa4d2a2658f6fa9, but the fact that the Location IDL
> already contained a [Unforgeable] stringifier overrode that default behavior.

Filed https://github.com/whatwg/html/issues/2284.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yeah, so basically the spec changed on us after we'd been shipping what the previous spec said for a good long while.

How confidend are we that the spec is stable this time?  I'm loath to invest any time in this, or ask anyone else to, without some sort of guarantees on this front.  In any case, the bits that are needed to fix this are:

1)  Ignore what the spec is talking about with toString, because it's bonkers, per comment 1.
2)  Remove the JSPROP_ENUMERATE from http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/bindings/Codegen.py#3548-3550
3)  Remove the JSPROP_ENUMERATE from http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/bindings/Codegen.py#2487-2495

That will fix all the bits except window.location.valueOf === Object.prototype.valueOf testing false.  For that one... first note that currently the two valueOf methods have different behavior.  Here's a simple example; these expressions both test true per the old Location spec:

  location.valueOf.call(5) === 5
  Object.prototype.valueOf.call(5) !== 5

Again, it's not clear to me whether this behavior change is desirable.  I should note that other browsers (e.g. Chrome) don't seem to be in a hurry to implement the new thing: location.valueOf !== Object.prototype.valueOf there.

In any case, if one did want to implement this, it would be by far the hardest part of this bug; possibly worth doing in a separate bug.  Object.prototype.valueOf is not self-hosted, so the usual way we common up methods by listing the selfHostedName wouldn't work.  SpiderMonkey doesn't seem to have a declarative way of saying you want a particular JSNative-backed built-in.  So whoever wants to fix this part would need to either add such a way or self-host Object.prototype.valueOf.

In either case, once all that's is done you'd want to remove http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/dom/bindings/BindingUtils.cpp#1214-1220

Comment 4

[Anne (:annevk)]

window.location.valueOf === Object.prototype.valueOf and location.valueOf.call(5) !== 5 are true in Safari TP. They implemented the standard recently.

The standard has been discussed with you, Bobby, and Adam (Google), and hopefully also reviewed to some extent.

valueOf has been discussed at https://github.com/whatwg/html/pull/638#issuecomment-185890302 and you were involved in that too.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yes, I'm aware of all that.  The real question is what Blink and WebKit plan to do here.

Comment 6

[Anne (:annevk)]

Steps I've taken thus far:

* Provide a fix for the toString issue in HTML: https://github.com/whatwg/html/pull/2294
* Wrote (additional) tests for toString, toJSON, valueOf, and Symbol.toPrimitive: https://github.com/w3c/web-platform-tests/pull/4623

Once those are reviewed I will file issues against the other browsers. We can then either await what they do or try to fix this first.

Comment 7

[Anne (:annevk)]

The issue with toString() in the HTML Standard is now fixed. It also resulted in us removing toJSON() from Location as only Gecko had that implemented. The tests have been updated accordingly and landed on WPT.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Update Location object properties to current spec

Specifically, three changes:

1) valueOf should be non-enumerable.
2) valueOf should be === to Object.prototype.valueOf.
3) There should be no toJSON.

The tests come directly from https://github.com/w3c/web-platform-tests/pull/4623
so not much need to review them.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5061e0dedf2f
Update Location object properties to current spec.  r=qdot

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5061e0dedf2f