Change exception thrown when popover attribute is not present

[josepharhar]

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

• At least two implementers are interested (and none opposed):
  • WebKit
  • Chromium
• Tests are written and can be reviewed and commented upon at:
  • Reverting WebKit export of https://bugs.webkit.org/show_bug.cgi?id=252102 web-platform-tests/wpt#38512 which happened in Rip out all of the beautiful bespoke Popover animation stuff web-platform-tests/wpt#38392 ends up testing this.
• Implementation bugs are filed:
  • Chromium: N/A
  • Gecko: N/A
  • WebKit: ?
• MDN issue is filed: N/A (presumably)

(See WHATWG Working Mode: Changes for more details.)

---

/popover.html ( diff )

[annevk]

We discussed these exceptions as part of the popover PR pretty explicitly. I think the status quo is fine here.

cc @nt1m

[annevk]

I missed that you had coordinated this with @nt1m already. As I don't feel strongly I've done a quick editorial pass.

[nt1m]

@annevk I'd like us to go with whatever exception is more consistent with prior art, whichever that is.

[annevk]

@nt1m I'm afraid exceptions are all over the map.

[mfreed7]

Just to add my logic for why they're the way they're currently implemented in Chromium (and described in this spec PR):

• NotSupported seems to make the most sense for calling showPopover() on an element that "isn't a popover" (doesn't wear the popover attribute), so that operation is "not supported".
• InvalidState makes sense when showPopover() is called on a popover that isn't in the "right state", such as when it is already in the open state.

I'm ok either way really, this just made semantic sense to me.

[annevk]

I pushed some nits. Let me know what you think.

@mfreed7 did you create a PR to revert the test changes?

[josepharhar]

It looks like the changes were already reverted here: web-platform-tests/wpt#38392

[mfreed7]

It looks like the changes were already reverted here: web-platform-tests/wpt#38392

Yep that’s right - I reverted the exception changes as part of that larger PR.

[annevk]

I'm going to assume my nits are okay.

[annevk]

@nt1m do we have a bug to track this or do I/you need to file something?