Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cancel event doesn't bubble #1212

Closed
dontcallmedom opened this issue Apr 16, 2024 · 10 comments
Closed

cancel event doesn't bubble #1212

dontcallmedom opened this issue Apr 16, 2024 · 10 comments

Comments

@dontcallmedom
Copy link
Member

see https://github.com/orgs/mdn/discussions/665#discussioncomment-9125774
@tidoust
Copy link
Member

tidoust commented Apr 16, 2024

But the spec says it does, at least for input elements?
https://html.spec.whatwg.org/multipage/input.html#common-input-element-apis:event-cancel

@dontcallmedom
Copy link
Member Author

but not on dialog elements? https://html.spec.whatwg.org/multipage/interactive-elements.html#the-dialog-element:event-cancel

if so, not sure how we can represent this usefully…

@dontcallmedom
Copy link
Member Author

hmm... I misread, it's on the CloseWatcher interface that it doesn't bubble (it couldn't anyway)

@dontcallmedom
Copy link
Member Author

now there is a question as to why we don't detect the cancel event on CloseWatcher

@dontcallmedom
Copy link
Member Author

sorry, I'm confusing two data points:

@tidoust
Copy link
Member

tidoust commented Apr 16, 2024

now there is a question as to why we don't detect the cancel event on CloseWatcher

That's because the data-dfn-for attribute needs fixing in the definition of cancel in the HTML spec. It should be data-dfn-for="CloseWatcher,HTMLElement" and not only data-dfn-for="HTMLElement".

(Same problem in the definition of the close event apparently, which should have data-dfn-for="CloseWatcher,HTMLElement,MessagePort").

@dontcallmedom
Copy link
Member Author

I guess we don't have but could benefit from a system that detects EventTarget interfaces for which is there is no matching known events?

@tidoust tidoust mentioned this issue Apr 16, 2024
Open
@tidoust
Copy link
Member

tidoust commented Apr 16, 2024

Yes, I was thinking about that as well. I created #1216 to track this.

dontcallmedom added a commit to dontcallmedom/html that referenced this issue Apr 17, 2024
@dontcallmedom
Complete machine-readable scope of and event dfns
1744635 
The summary table correctly lists `CloseWatcher` (for `cancel` and `close`) and
`MessagePort` (for `close`) as target interfaces for the event, but these
interfaces are missing from the machine-readable `for` attribute.

Via w3c/webref#1212 (comment)
@dontcallmedom dontcallmedom mentioned this issue Apr 17, 2024
Merged
domenic pushed a commit to whatwg/html that referenced this issue Apr 18, 2024
@dontcallmedom
Meta: add some missing for="" values for events
c42d28e 
See w3c/webref#1212 (comment).
@tidoust
Copy link
Member

tidoust commented Apr 19, 2024

if so, not sure how we can represent this usefully…

An update following discussion and some playing with code. In the consolidated view, the bubbling information is already per target interface, so we don't need to change the structure to represent an event that bubbles for some set of target interfaces and does not bubble for another set. The only problem is that the consolidation code cannot yet produce the right information from an events extract because...

The bubbling information is per entry in an events extract. To represent an event that bubbles or not depending on the target interface without changing the structure of the extract, we're going to have to duplicate the entry. That seems relatively fine, we don't guarantee that a given event type will appear only once in an events extract, and known and would-be consumers are going to use the consolidated view in practice.

I'm about to raise a PR on Reffy to have the events post-processor consolidate such duplicate entries into the right structure.

And then I'll prepare a patch for the cancel event in Webref to (adjust the target interfaces and) create the duplicate in the events extract, which will then be consolidated in the right entry.

tidoust added a commit to w3c/reffy that referenced this issue Apr 19, 2024
@tidoust
Events post-processor: consolidate events that "babble"
a0ddffd 
It seems possible to end up with events that bubble for some target interfaces
but don't for other target interfaces, see discussion in:
w3c/webref#1212

The post-processor already knew how to assign a bubbling flag per target
interface because we make a distinction between interfaces that are in a
bubbling tree and interfaces that are not.

However, the post-processor did not know how to consolidate an entry that
appears duplicated in the events extract because the underlying event bubbles
in one case but not in the other. This update makes it able to consolidate
these events (which I call "babbling" events because, hey, it's Friday
evening).

We still don't have post-processing tests but that seems to work fine on Webref
data.
@tidoust tidoust mentioned this issue Apr 19, 2024
Merged
tidoust added a commit to w3c/reffy that referenced this issue Apr 19, 2024
@tidoust
Events post-processor: consolidate events that "babble" (#1546)
e7bcbdc 
It seems possible to end up with events that bubble for some target interfaces
but don't for other target interfaces, see discussion in:
w3c/webref#1212

The post-processor already knew how to assign a bubbling flag per target
interface because we make a distinction between interfaces that are in a
bubbling tree and interfaces that are not.

However, the post-processor did not know how to consolidate an entry that
appears duplicated in the events extract because the underlying event bubbles
in one case but not in the other. This update makes it able to consolidate
these events (which I call "babbling" events because, hey, it's Friday
evening).

We still don't have post-processing tests but that seems to work fine on Webref
data.
tidoust added a commit that referenced this issue Apr 19, 2024
@tidoust
Patch cancel event to refine bubbling information
c4c2cb8 
As discussed in #1212, the `cancel` event bubbles on `HTMLInputElement`,
but it does not bubble on `HTMLDialogElement`. It does not bubble on
`CloseWatcher` either but that's because `CloseWatcher` is not part of a
bubbling tree in any case.

Events extraction cannot automatically get this nuance, be it only because it
only sees `HTMLElement` as target interface for the event, and not
`HTMLInputElement` and `HTMLDialogElement`. In practice, it claims that the
event bubbles, which is neither wrong nor right.

This patch restricts the `cancel` entry in the HTML events extract to only
target `HTMLInputElement`, in order to create the right bubbling entry. And
it creates another `cancel` entry which does not bubble for `HTMLDialogElement`
and `CloseWatcher`.

With this patch (and the new version of Reffy), we should end up with the
following entry in the consolidated `events.json` file, which captures the
fact that:
- the event bubbles on `HTMLInputElement`
- the event does not bubble on `HTMLDialogElement`
- the concept of bubbling is meaningless on `CloseWatcher`

```json
{
  "href": "https://html.spec.whatwg.org/multipage/indices.html#event-cancel",
  "src": {
    "format": "summary table",
    "href": "https://html.spec.whatwg.org/multipage/indices.html#event-cancel"
  },
  "type": "cancel",
  "targets": [
    {
      "target": "HTMLInputElement",
      "bubbles": true,
      "bubblingPath": [
        "Node",
        "Document",
        "Window"
      ]
    },
    {
      "target": "CloseWatcher"
    },
    {
      "target": "HTMLDialogElement",
      "bubbles": false
    }
  ],
  "interface": "Event"
}
```
@tidoust tidoust mentioned this issue Apr 19, 2024
Merged
tidoust added a commit that referenced this issue Apr 22, 2024
@tidoust
Patch cancel event to refine bubbling information (#1220)
8a4f7c4 
As discussed in #1212, the `cancel` event bubbles on `HTMLInputElement`,
but it does not bubble on `HTMLDialogElement`. It does not bubble on
`CloseWatcher` either but that's because `CloseWatcher` is not part of a
bubbling tree in any case.

Events extraction cannot automatically get this nuance, be it only because it
only sees `HTMLElement` as target interface for the event, and not
`HTMLInputElement` and `HTMLDialogElement`. In practice, it claims that the
event bubbles, which is neither wrong nor right.

This patch restricts the `cancel` entry in the HTML events extract to only
target `HTMLInputElement`, in order to create the right bubbling entry. And
it creates another `cancel` entry which does not bubble for `HTMLDialogElement`
and `CloseWatcher`.

With this patch (and the new version of Reffy), we should end up with the
following entry in the consolidated `events.json` file, which captures the
fact that:
- the event bubbles on `HTMLInputElement`
- the event does not bubble on `HTMLDialogElement`
- the concept of bubbling is meaningless on `CloseWatcher`

```json
{
  "href": "https://html.spec.whatwg.org/multipage/indices.html#event-cancel",
  "src": {
    "format": "summary table",
    "href": "https://html.spec.whatwg.org/multipage/indices.html#event-cancel"
  },
  "type": "cancel",
  "targets": [
    {
      "target": "HTMLInputElement",
      "bubbles": true,
      "bubblingPath": [
        "Node",
        "Document",
        "Window"
      ]
    },
    {
      "target": "CloseWatcher"
    },
    {
      "target": "HTMLDialogElement",
      "bubbles": false
    }
  ],
  "interface": "Event"
}
```
@tidoust
Copy link
Member

tidoust commented Apr 22, 2024

Closing as addressed. The cancel event entry now correctly reports that the event bubbles on HTMLInputElement, does not bubble on HTMLDialogElement, and also fires at CloseWatcher where bubbling does not mean anything. The update was released in v1.11.0 of @webref/events.

@tidoust tidoust closed this as completed Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dontcallmedom @tidoust