New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 683310 link

Starred by 3 users
Status: Fixed
Owner:
█ j...@opera.com
Last visit > 30 days ago
Closed: May 2017
Cc:
yukishiino@chromium.org
bashi@chromium.org
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

PutForwards implementation doesn't follow the spec

Reported by bzbar...@mit.edu, Jan 20 2017 
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:53.0) Gecko/20100101 Firefox/53.0

Example URL:

Steps to reproduce the problem:
1. Load attached testcase

What is the expected behavior?
An alert saying "CALLED"

What went wrong?
No alert.

Does it occur on multiple sites: Yes

Is it a problem with a plugin? No 

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: 57.0.2986.0 (Official Build) dev (64-bit)  Channel: n/a
OS Version: OS X 10.12
Flash Version: 

Relevant spec text from https://heycam.github.io/webidl/#dfn-attribute-setter step 9:

If attribute is declared with a [PutForwards] extended attribute, then:

    Let Q be ? Get(O, id).

    If Type(Q) is not Object, then throw a TypeError.

    Let forwardId be the identifier argument of the [PutForwards] extended attribute.

    Perform ? Set(Q, forwardId, V).

    Return undefined.

in this case the testcase is checking that Get(O, id) actually happens.  Doesn't look like it does in Chrome.
baz.html
579 bytes View Download

Comment 1 by domenic@chromium.org, Jan 20 2017

Components: -Blink Blink>Bindings

Comment 2 by bashi@chromium.org, Jan 23 2017

Cc: bashi@chromium.org yukishiino@chromium.org
Status: Available (was: Unconfirmed)

Comment 3 by j...@opera.com, Mar 3 2017

Owner: j...@opera.com
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, May 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0fda810e000bd150860e33350b4a589d4acda1f9

commit 0fda810e000bd150860e33350b4a589d4acda1f9
Author: jl <jl@opera.com>
Date: Thu May 11 07:39:58 2017

Reimplement [PutForwards] per spec

An attribute setter for an attribute X with [PutForwards=Y] should mostly
just do

  Set(Get(this, X), Y, value)

With the previous implementation, we instead essentially inlined both the
getting of X and setting of Y into the setter. This is unnecessary (both
will be implemented correctly separately) and also incorrect, since both
the getter for X and setter for Y could be overridden by a script.

BUG= 683310 

Review-Url: https://codereview.chromium.org/2733763003
Cr-Commit-Position: refs/heads/master@{#470864}

[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/LayoutTests/fast/frames/sandboxed-iframe-navigation-top-denied-expected.txt
[add] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/LayoutTests/fast/js/put-forwards.html
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/templates/attributes.cpp.tmpl
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8SVGTestInterface.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestCallbackFunctions.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexed.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedGlobal.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedPrimaryGlobal.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceCheckSecurity.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceDocument.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceGarbageCollected.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceNode.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceOriginTrialEnabled.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceSecureContext.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestNode.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/core/V8TestTypedefs.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.cpp
[modify] https://crrev.com/0fda810e000bd150860e33350b4a589d4acda1f9/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterfacePartial.cpp

Comment 5 by j...@opera.com, May 11 2017

Status: Fixed (was: Started)

Comment 6 by domenic@chromium.org, May 11 2017 

Would be ideal if third_party/WebKit/LayoutTests/fast/js/put-forwards.html could be moved to third_party/WebKit/LayoutTests/external/wpt/WebIDL/ecmascript-bindings/put-forwards.html

Comment 7 by j...@opera.com, May 11 2017 

domenic: I'll look into moving it there.
►

Sign in to add a comment