WebKit export of https://bugs.webkit.org/show_bug.cgi?id=252102 #38512
Conversation
The spec says to use InvalidStateError not NotSupportedError
wpt-pr-bot
requested review from
annevk,
domenic,
foolip,
jdm,
jgraham and
zqzhang
|
cc @josepharhar @mfreed7 to fix Chrome's implementation to match the spec. |
|
Ah this was actually due to me messing up the spec, it was supposed to be NotSupportedError in the spec. The reasoning is that showPopover() is not supported on elements without the popover attribute, whereas InvalidStateError is thrown when the popover attribute is present but there's something else in the wrong state, such as the popover already showing when showPopover() is called. Are you OK with me changing the spec? |
|
@josepharhar InvalidStateError and NotSupportedError both make sense to me, I'm fine with whatever is more consistent with prior art. |
This patch changes the exception thrown from InvalidStateError to NotSupportedError when showPopover or hidePopover is called on elements which don't have a popover attribute. Chrome's implementation does this and I missed it when I initially wrote the spec. The reason that NotSupportedError is thrown instead is that elements wich don't have a popover attribute don't support showPopover and hidePopover, and in the other cases showPopover and hidePopover are supported but something else is in the wrong state, such as the popover already showing when showPopover is called. This was raised here: web-platform-tests/wpt#38512
|
I opened a PR here: whatwg/html#8891 |
|
This PR missed one more at line 383. But given the discussion at whatwg/html#8891 I'm planning to revert the changes from this PR. I won't land it until we resolve that issue though. Hopefully ok! |
As discussed in web-platform-tests/wpt#38512. (Those test changes will be reverted.)
As discussed in web-platform-tests/wpt#38512. Those tests were reverted as part of web-platform-tests/wpt#38392. Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
The spec says to use InvalidStateError not NotSupportedError for popovers.
The spec says to use InvalidStateError not NotSupportedError for popovers.