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.

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

Reland "Update popover post-toggle event naming and behavior" #38269

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jan 30, 2023

This is a reland of commit 2e5ee120d7fc5df4b6c101e88d03a5f62a73d8b8

It seems that the event coalescing behavior changes cause a segfault
in the event dispatch code. This reland (compare to Patchset 1) just
includes the event renaming, and not the new behavior. I'll land
the new behavior in a separate CL to make it smaller if it gets
reverted again. This part should be safe.

Original change's description:

Update popover post-toggle event naming and behavior

This CL updates the post-toggle event in the following ways:

  1. Rename the 'aftertoggle' event to 'toggle'.
  2. Rename PopoverToggleEvent to ToggleEvent.
  3. Rename the currentState attribute to oldState.
  4. Add event coalescing behavior. If two transitions occur before the
    first 'toggle' event has been fired, cancel the first event and
    queue a replacement that has oldState === newState.

These changes were driven by the corresponding changes to the spec PR:
whatwg/html#8717

Bug: 1307772
Change-Id: Iabc5a9093d7cef3bbd6e54e488d8e571c51ea568
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195120
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1098728}

Bug: 1307772
Change-Id: Iede18bbf05516cec4ee881f1afc663c45380c82e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4205710
Reviewed-by: Joey Arhar <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1099397}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

This is a reland of commit 2e5ee120d7fc5df4b6c101e88d03a5f62a73d8b8

It seems that the event coalescing behavior changes cause a segfault
in the event dispatch code. This reland (compare to Patchset 1) just
includes the event renaming, and not the new behavior. I'll land
the new behavior in a separate CL to make it smaller if it gets
reverted again. This part should be safe.

Original change's description:
> Update popover post-toggle event naming and behavior
>
> This CL updates the post-toggle event in the following ways:
>  1. Rename the 'aftertoggle' event to 'toggle'.
>  2. Rename PopoverToggleEvent to ToggleEvent.
>  3. Rename the currentState attribute to oldState.
>  4. Add event coalescing behavior. If two transitions occur before the
>     first 'toggle' event has been fired, cancel the first event and
>     queue a replacement that has oldState === newState.
>
> These changes were driven by the corresponding changes to the spec PR:
>   whatwg/html#8717
>
> Bug: 1307772
> Change-Id: Iabc5a9093d7cef3bbd6e54e488d8e571c51ea568
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195120
> Auto-Submit: Mason Freed <[email protected]>
> Commit-Queue: Joey Arhar <[email protected]>
> Reviewed-by: Joey Arhar <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1098728}

Bug: 1307772
Change-Id: Iede18bbf05516cec4ee881f1afc663c45380c82e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4205710
Reviewed-by: Joey Arhar <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1099397}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants