Bug 267900

Summary: InvalidStateError not thrown in Safari 17 when dialog showModal() called twice
Product: WebKit Reporter: Jeff Johnson <opendarwin>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: annevk, karlcow, mike, ntim, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 17   
Hardware: All   
OS: All   
Attachments:
Description Flags
Same HTML demonstrating the bug none

Jeff Johnson
Reported 2024-01-22 18:51:51 PST
Created attachment 469511 [details] Same HTML demonstrating the bug According to https://developer.mozilla.org/docs/Web/API/HTMLDialogElement/showModal#invalidstateerror InvalidStateError is thrown "if the dialog is already open (i.e. if the open attribute is already set on the <dialog> element), or if the dialog is also a popover that is already being shown." If showModal() is called twice on the same dialog, InvalidStateError is thrown in Safari 16 on iOS and Mac but not in Safari 17 on iOS and Mac.
Attachments
Same HTML demonstrating the bug (393 bytes, text/html)
2024-01-22 18:51 PST, Jeff Johnson
no flags
Radar WebKit Bug Importer
Comment 1 2024-01-22 19:46:23 PST
sideshowbarker
Comment 2 2024-01-30 01:35:16 PST
With the “showModal() on a <dialog> that already has an open attribute throws an InvalidStateError exception” test case from http://wpt.live/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html, I can’t reproduce this either in Safari 17.3 nor STP 187, https://wpt.fyi/results/html/semantics/interactive-elements/the-dialog-element/dialog-showModal.html also shows that test case passing in STP 187 and Safari 17.3 So it seems that for anybody to be able to investigate this issue further, what would be needed is a minimal reproducible example/reduction that demonstrates Safari not throwing when it should — because the existing test cases in WPT show Safari throwing as expected. https://stackoverflow.com/help/minimal-reproducible-example
Jeff Johnson
Comment 3 2024-01-30 04:25:00 PST
(In reply to sideshowbarker from comment #2) > So it seems that for anybody to be able to investigate this issue further, > what would be needed is a minimal reproducible example/reduction that > demonstrates Safari not throwing when it should — because the existing test > cases in WPT show Safari throwing as expected. There was already a minimal reproducible example attached to my bug report: https://bug-267900-attachments.webkit.org/attachment.cgi?id=469511 The difference between our test cases is that yours has an open dialog already in the HTML and calls showModal() only once, whereas mine calls showModal() twice, hence the title of this bug report.
sideshowbarker
Comment 4 2024-01-30 05:12:04 PST
(In reply to Jeff Johnson from comment #3) > (In reply to sideshowbarker from comment #2) > > So it seems that for anybody to be able to investigate this issue further, > > what would be needed is a minimal reproducible example/reduction that > > demonstrates Safari not throwing when it should — because the existing test > > cases in WPT show Safari throwing as expected. > > There was already a minimal reproducible example attached to my bug report: > https://bug-267900-attachments.webkit.org/attachment.cgi?id=469511 ah OK — sorry for not having noticed that. So I see that while that example doesn’t cause Safari 17.3 to throw, it also doesn’t cause Chrome or Firefox to throw either. Instead all three browsers log the “This should not be reached” message. > The difference between our test cases is that yours has an open dialog > already in the HTML and calls showModal() only once, whereas mine calls > showModal() twice, hence the title of this bug report. Since the behavior in Safari, Chrome, and Firefox is the same for your test case, it seems that either the test case doesn’t actually conform to the requirement stated in the spec — or else all three browser engines (not just WebKit) are failing to conform to the requirement. [I’m not stating one or the other — because I haven’t yet looked at what the relevant requirement is in the spec (which may be different from what that https://developer.mozilla.org/docs/Web/API/HTMLDialogElement/showModal#invalidstateerror MDN page states].
Tim Nguyen (:ntim)
Comment 5 2024-01-31 14:07:19 PST
This was an intentional spec changes that we implemented: https://github.com/whatwg/html/pull/9142 See https://bugs.webkit.org/show_bug.cgi?id=256167
Tim Nguyen (:ntim)
Comment 6 2024-01-31 14:08:47 PST
Firefox & Chrome should also have the same behavior as Michael pointed out.
Jeff Johnson
Comment 7 2024-01-31 14:54:19 PST
(In reply to Tim Nguyen (:ntim) from comment #5) > This was an intentional spec changes that we implemented: > https://github.com/whatwg/html/pull/9142 > > See https://bugs.webkit.org/show_bug.cgi?id=256167 I've read through the discussion in https://github.com/whatwg/html/issues/9045 and now I'm confused about why showModal still throws in the case that sideshowbarker mentioned, when the <dialog> already has an open attribute in the HTML? Also, shouldn't the MDN docs be changed?
Tim Nguyen (:ntim)
Comment 8 2024-01-31 15:42:25 PST
(In reply to Jeff Johnson from comment #7) > (In reply to Tim Nguyen (:ntim) from comment #5) > > This was an intentional spec changes that we implemented: > > https://github.com/whatwg/html/pull/9142 > > > > See https://bugs.webkit.org/show_bug.cgi?id=256167 > > I've read through the discussion in > https://github.com/whatwg/html/issues/9045 and now I'm confused about why > showModal still throws in the case that sideshowbarker mentioned, when the > <dialog> already has an open attribute in the HTML? > The case sideshowbarker mentioned was dialog.show() followed by dialog.showModal() I think, which is invalid because you're trying to promote a normal dialog to a modal dialog. > Also, shouldn't the MDN docs be changed? Probably yes, I'd suggest filing an issue at: https://github.com/mdn/content
Jeff Johnson
Comment 9 2024-01-31 16:03:07 PST
> The case sideshowbarker mentioned was dialog.show() followed by > dialog.showModal() I think, which is invalid because you're trying to > promote a normal dialog to a modal dialog. It was actually a dialog element with the open attribute already set in the HTML, but I guess that's effectively the same. It seems to me that the entire dialog API is very confusing, because a prexisting open attribute means the dialog is non-modal, but calling showModal on a non-open dialog also sets the open attribute! Moreover, there's no "modal" attribute, even though according to the spec there appears to be an internal "is modal" flag.
Note You need to log in before you can comment on or make changes to this bug.