Interesting bug!

This change was caused by https://chromium.googlesource.com/chromium/src/+/97235496c3dee67efb24f9a7893232df493dcd1e

I think it's intended behavior, as you're now hitting the slot getter.

Confirmed that this is also broken in Safari Technology Preview (which has shipped the slot-based Shadow DOM API), and works in current Safari (which has not).

Wow, I am surprised.  We came across this because it broke a web page that used the variable name "slot" to store an object of type googletag.Slot, as returned by the doubleclick tag API method googletag.defineSlot() -- see https://developers.google.com/doubleclick-gpt/reference.  This API is used by a large number of web sites, and I have to assume the variable name "slot" is not an uncommon choice.

In case anyone’s wondering, a global “slot” variable seems to work fine in event handlers bound with the addEventListener API instead of the onclick attribute.

Live demo: https://plnkr.co/edit/Uw28kUT5YOfv5MtZZA2M?p=preview

This in an unintentional behavior.

It looks that "slot" is not only the attribute which causes this issue.
I can reproduce the similar behavior by "style".

See https://jsfiddle.net/hayatoito/y3027rro/.

I am now looking for a spec which defines the behavior in these cases.

Though I am still looking for a spec, but firefox has the same behavior for "style". Given that, it might be a spec-compliant behavior.

It looks this behavior is spec-compliant.

Per https://html.spec.whatwg.org/#getting-the-current-value-of-the-event-handler

> Report the error for the appropriate script and with the appropriate position (line number and column number) given by location, using the global object specified by script settings as the target. If the error is still not handled after this, then the error may be reported to a developer console.

The spec requires: "using global object specified by script setting as the target".

The spec around here is hard to read, but I guess that is the element, in this case.
Thus, element's attribute name wins in event's handler.

 

Yes, this is indeed per-spec. Unfortunately, it looks like might be not Web-compatible. This happens occasionally (remember rootNode?). While "style" attribute does exhibit the same behavior, it was there for a while, so it's something Web developers are used to. They are clearly aren't used to "slot", and it looks like this breaks content in the wild. This affects negatively the predictability of the Web platform.

Now we need to weigh the costs/benefits of this change and decide whether we should forge ahead and mitigate the change with outreach, or change the name to be less collision-prone. In either case, this warrants a spec bug (just like rootNode did).

Adding rbyers as your predictability guide.

Ah, I agree. Let's keep this bug open, and use this bug to collect all web sites which might be broken by introducing global slot attributes.

Since Chrome M53 is just rolling out, I'm afraid that we do not have enough data yet to judge costs/benefits.

I might be in a position to help gauge the impact.

Is it possible for JS to modify the prototype of the shadowDOM-related slot thing that is shadowing window.slot?  If so, I could instrument pages using our JS to show a console error message (pointing at this crbug) any time a method from our API gets invoked on the wrong thing named "slot".

If nothing else, I guess I could add a window.onerror that matched against /Uncaught TypeError: slot\.[\w]* is not a function/.  But I'd like to think we live in a more civilized age (...where there's a more elegant weapon).

I'm afraid it is difficult to measure the impact, exactly, from user-land JavaScript.

For example:

1. console.log(div.slot);
2. <div onclick="console.log(slot);">

Even if we can *hijack* "something.__prototype__.slot" somehow, and inject some code, for example, function `f1`,
it is still difficult to tell whether `f1` is called as a result of accessing 'slot' in "onclick" (case 2), or not (case 1)

I think we should WontFix this.

Inline event handlers stuff things on the scope chain (think JS "with"); that's where 'slot' is getting mixed in here. Comment 8 is right:

https://html.spec.whatwg.org/#the-event-handler-processing-algorithm calls
https://html.spec.whatwg.org/#getting-the-current-value-of-the-event-handler

And step 1.9 is the important one. It's as if event handlers ran like this:

with (element.ownerDocument) {
  with (element.form) { // *if* form associated: image, object, input, ...
    with (element) {
      ...
    }
  }
}

Browsers have always done it this way. It's surprising, I guess.

If you want to refer to window.slot, then write window.slot (not 'slot') and obviously don't stuff a random 'window' property on elements :)

I guess devtools could warn you if you have globals/window properties with names that overlap anything in document or an element.

Any kind of runtime check is probably slow (I guess we'd need to look at caller frames in element property getters?) and not that effective (with the way V8 is set up now we couldn't easily tell if you wrote event.target.slot, which probably means you did want the element's slot, or just slot, which could mean you knew about the scope chain thing and wanted element.slot or you didn't know about it and we should warn you.)

Yeah, it is difficult to measure the exact impact without sacrificing a performance. I think that it is not worth doing.

The remaining *issue* is how much a 'slot' attribute might break an existing Web, as Dimitri said in #9.

I don't care whether this issue is kept open or closed, however, I would like to use this bug to keep track of broken web sites, if any, to collect data to judge. If someone gets a report of a broken web site by slot attributes, please feel free to report it and re-open this issue.

OK, we can dedup things broken by 'slot' into this bug.

Do we want to keep track of things broken by "new API" in general? I remember an early version of custom elements which used document.register broke a page which used a 'register' property on window.

I think it is on a case-by-case basis.

In this case, I think the risk is low, and we don't have to spend our time on finding broken sites. Just doing nothing and waiting for a incoming report is good enough, I think.

Couldn't this also be solved by making `slot` [Unscopable]? That is largely why we invented unscopable, after all.

\o/

\o/
Let me try it in Blink! It it would work well, we might want to mention it HTML Standard.

Yes, I have verified [Unscopeable] solves this issue. Kudo for domenic@!

WIP CL is: https://codereview.chromium.org/2343783002.
PR for DOM Standard: https://github.com/whatwg/dom/pull/329

BTW, it looks Blink's IDL is using *Unscopeable*, rather than *Unscopable*. :(

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

commit 7db5d42ee2012ec61580506af7c0233fd049628d
Author: hayato <hayato@chromium.org>
Date: Thu Sep 15 08:52:54 2016

Make slot attributes unscopable

PR for DOM Standard is: https://github.com/whatwg/dom/pull/329

Note: Blink's IDL uses [Unscopeable], instead of [Unscopable].
I have filed a bug: https://bugs.chromium.org/p/chromium/issues/detail?id=647133.

Let me omit to add a layout test. It should be covered in WPT.

BUG= 643869 

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

[modify] https://crrev.com/7db5d42ee2012ec61580506af7c0233fd049628d/third_party/WebKit/Source/core/dom/Element.idl