Re: [whatwg/dom] Add window.event, event.srcElement, event.returnValue as a legacy compat requirements (#407)

annevk commented on this pull request.

I decided to review anyway to allow for parallel addressing of comments. Hope that helps.

> @@ -499,6 +505,15 @@ list of tuples, each of which consists of an <b>item</b> (an {{EventTarget}} obj
 object). A tuple is formatted as (<b>item</b>, <b>target</b>, <b>relatedTarget</b>). A <a
 for=Event>path</a> is initially the empty list.</p>
 
+For web compatibility, the <a>current global object</a> has an

All new/changed paragraphs should start with `<p>`.

This paragraph should be reworded and moved done I think. It makes more sense to me to define this extension to the `Window` object at the end of this section since it's also at the end of the IDL.

Furthermore, we shouldn't give the reason in the description of the feature. That can go into a note if it's important.

> @@ -499,6 +505,15 @@ list of tuples, each of which consists of an <b>item</b> (an {{EventTarget}} obj
 object). A tuple is formatted as (<b>item</b>, <b>target</b>, <b>relatedTarget</b>). A <a
 for=Event>path</a> is initially the empty list.</p>
 
+For web compatibility, the <a>current global object</a> has an
+<dfn attribute for=Window><code>event</code></dfn> attribute that returns the
+<dfn export id=concept-current-event>current event</dfn>, which is initially
+set to undefined.
+
+<p class="note no-backref">Authors <span class=allow-2119>should</span> explicitly pass in the
+event object to event listener callbacks, rather than rely on a global <code>window.event</code>
+object.

I wouldn't use should here. Just say "strongly encouraged".

> @@ -684,6 +700,13 @@ The <dfn attribute for=Event><code>bubbles</code></dfn> and
 <dfn attribute for=Event><code>cancelable</code></dfn> attributes
 must return the values they were initialized to.
 
+The <dfn attribute for=Event><code>returnValue</code></dfn>
+attribute must be initialized to true when an <a>event</a> is created.

I don't think this makes sense. You want to instead describe this as a getter and a setter. The default value is already evident from the default value of the canceled flag after all.

See https://url.spec.whatwg.org/#dom-url-protocol for an example of defining a getter and a setter.

> @@ -1357,6 +1380,9 @@ with <var>event</var>, <var>listeners</var>, and an optional
    <li><p>If <var>listener</var>'s <b>passive</b> is true, then set <var>event</var>'s
    <a>in passive listener flag</a>.
 
+   <li><p>If <var>event</var>'s {{Event/target}} <a for=tree>root</a> is not a
+   <a for=/>shadow root</a>, set the <a>current event</a> to <var>event</var>.

then set*

> @@ -499,6 +505,15 @@ list of tuples, each of which consists of an <b>item</b> (an {{EventTarget}} obj
 object). A tuple is formatted as (<b>item</b>, <b>target</b>, <b>relatedTarget</b>). A <a
 for=Event>path</a> is initially the empty list.</p>
 
+For web compatibility, the <a>current global object</a> has an
+<dfn attribute for=Window><code>event</code></dfn> attribute that returns the
+<dfn export id=concept-current-event>current event</dfn>, which is initially
+set to undefined.

You also need to define what current event is associated with. Also, we should probably test that this quirk is not present in workers? And ensure the algorithms work in a worker context as well, since events don't just dispatch inside a window.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/pull/407#pullrequestreview-97484829