Closed Bug 1876762 Opened 2 years ago Closed 1 year ago

[meta] Implement dialog toggleevents for dialog open/close

Categories

(Core :: DOM: Core & HTML, enhancement)

Firefox 121
enhancement

Tracking

()

RESOLVED FIXED
133 Branch
Tracking Status
relnote-firefox --- 133+
firefox133 --- fixed

People

(Reporter: keithamus, Assigned: keithamus)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, meta)

Attachments

(1 file, 1 obsolete file)

Hello! Thank you for submitting this issue but could you please post the stepts to reproduce here instead of just the link to git for better visibility of the issue.

Have a nice day!

Flags: needinfo?(mozilla)
Blocks: html
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core

This is a tracking issue for implementing beforetoggle and toggle events on <dialog> elements; as proposed in https://github.com/whatwg/html/issues/9733, which was discussed at TPAC. https://github.com/whatwg/html/pull/10091 is the spec PR for this.

Currently <dialog> elements do not emit these events.

Hope that's helpful.

Flags: needinfo?(mozilla)
Summary: Implement dialog toggleevents for dialog open/close → [meta] Implement dialog toggleevents for dialog open/close
Attachment #9384523 - Attachment is obsolete: true
Assignee: nobody → mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Depends on: 1876597

https://github.com/whatwg/html/pull/10091

This also slightly refactors FireToggleEvent to allow to to
accomodate both popovers and now also dialogs

Pushed by mozilla@keithcirkel.co.uk: https://hg.mozilla.org/integration/autoland/rev/a8b51403fa4f Implement togggle events for Dialog show/showModal/close r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48601 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 133 Branch
Upstream PR merged by moz-wptsync-bot

Keith, do you want to nominate this for a release note? (Process info)

Flags: needinfo?(mozilla)

(In reply to Donal Meehan [:dmeehan] from comment #9)

Keith, do you want to nominate this for a release note? (Process info)

This is still flagged; so maybe better to delay and add to release notes when it's unflagged?

Flags: needinfo?(mozilla)

(In reply to Keith Cirkel from comment #10)

(In reply to Donal Meehan [:dmeehan] from comment #9)

Keith, do you want to nominate this for a release note? (Process info)

This is still flagged; so maybe better to delay and add to release notes when it's unflagged?

I saw the value in the dom.element.dialog.toggle_events.enabled pref is True, sorry for the noise if it's still gated off somehow

Good point! Forgot about that :p. Hope this is sufficient:

Release Note Request (optional, but appreciated)
[Why is this notable]:
Firefox will now dispatch beforetoggle events just before a dialog is opening and toggle events after the dialog is closing, matching the behaviour of popovers.
[Affects Firefox for Android]:
[Suggested wording]:
Firefox will now dispatch beforetoggle events just before a dialog is opening and toggle events after the dialog is closing, matching the behaviour of popovers.
[Links (documentation, blog post, etc)]: https://bugzilla.mozilla.org/show_bug.cgi?id=1876762

relnote-firefox: --- → ?
Flags: needinfo?(dmeehan)

Thanks, added to the Fx133 nightly release notes, please allow 30 minutes for the site to update.
Keeping the relnote-firefox flag as ? to keep it on the radar for inclusion in the final Fx133 release notes.

Flags: needinfo?(dmeehan)
Depends on: 1925252

FF133 MDN docs for this can be tracked in https://github.com/mdn/content/issues/36536

Can you confirm the following:

  1. The implementation of the events is/will be the same for dialogs and popups.
  2. beforetoggle
    • is fired just before opening or closing the element.

    • when opening (only) the event is cancellable, so you could do this and stop the dialog from opening

      mydialog.addEventListener('beforetoggle', (event) => {
          // Check if you want to cancel the toggle
          if (shouldCancel()) {
              event.preventDefault();
          }
      });
      
    • when closing the event still fires but you can't prevent it closing. Presumably preventDefault is ignored.

    • Is there any other reason you might want to use beforetoggle?

  3. toggle is fired just after opening or closing the element and isn't cancellable
  4. The events are defined on HTMLElement and inherited.
  5. <details> currently also has a toggle event which is fired after open and close?
    • That's also on HTMLElement right (I ask because MDN appears to wrongly add it to HTMLDetailsElement.
    • I understand you plan to add another beforetoggle for `<details> as well. That will also be same definition in spec?
Flags: needinfo?(mozilla)

All of those statements sound correct to me.

As for

Is there any other reason you might want to use beforetoggle?

Applying classes to other elements (eg body) which start animations or set properties. Or pushing URL state so that a dialogs openness is part of the URL.

Flags: needinfo?(mozilla)

Docs work for this done - just trickling through last reviews. Thank you for all your help and advice @Keith!

Added to the Fx133 release notes.
Available to preview on staging, please allow 30 minutes for the site to update.

Blocks: 1963839
You need to log in before you can comment on or make changes to this bug.