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

[popup] The popup IDL attribute might not be web compatible #546

Closed
mfreed7 opened this issue Jun 9, 2022 · 12 comments
Closed

[popup] The popup IDL attribute might not be web compatible #546

mfreed7 opened this issue Jun 9, 2022 · 12 comments
Labels
popover The Popover API

Comments

@mfreed7
Copy link
Collaborator

mfreed7 commented Jun 9, 2022

A bug was filed on the Chromium implementation that might indicate some Web Compat issues for the popup IDL (Javascript) attribute. Because we resolved (here and here) that the popup attribute should reflect only valid (string) values, and should return null for invalid values, sites will break that already use popup as a property:

const myElement = document.querySelector('foo');
myElement.popup = myBigComplicatedObject;
myElement.getAttribute('popup'); // "[object Object]"
myElement.popup; // null
myElement.popup.runMethod(); // Breaks, because popup is null

I'm not sure what the best solution is for this problem. The obvious one is to rename popup (the IDL attribute, not the content attribute) to something less common. And the downside of that is that it is confusing for developers that myObject.newPopupAttrribute='auto' results in <div popup="auto">. Another would be to revert to myPopup.popup accepting any value. To feature-detect the values of popup, we'd then need another mechanism, such as a different/new property: myPopup.popupValidated or something like that.

Suggestions?

@mfreed7 mfreed7 added agenda+ Use this label if you'd like the topic to be added to the meeting agenda popover The Popover API labels Jun 9, 2022
@domenic
Copy link
Contributor

domenic commented Jun 10, 2022

Another would be to revert to myPopup.popup accepting any value.

The "normal" version of this, where you just remove "limited to only known values" from the spec, won't work. That would just convert el.popup = { ... some object ... } to el.popup = "[object Object]", which will break the site in the same way. I.e., all element reflection does string conversion; it doesn't matter whether you're limited to only known values, or not. You'd need to do something more dramatic.

Ideas:

  1. Push through the compat issue. Doesn't seem promising since there are many JIRA instances in the wild.

  2. Rename both the content and IDL attribute. toplayer=""/topLayer comes to mind! Also popupbehavior=""/popupBehavior... consider getting creative with suffixes/prefixes/etc. I know bikeshedding sucks but it might just be the best solution in this case.

  3. Introduce an asymmetry between the content and IDL attribute. E.g. popup=""/htmlPopup, similar to for=""/htmlFor or value=""/defaultValue. Highly disrecommended; developers hate this and it's something we'd prefer to never do going forward. IMO this is strictly worse than (2).

  4. Introduce huge hacks, such as, if the value being assigned is an object, overwrite the setter (similar to [Replacable] but only if the given value is an object). This basically means JIRA (and other pages like it) can never use the popup IDL attribute, and more importantly libraries probably won't be able to assume that it exists, because some page might have done the dumb thing and overwritten it.

(2) looks like by far the best option to me.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jun 13, 2022

Thanks for brainstorming ideas. I think I agree that #1 and #4 are likely off the table. In particular, #1 - this was found very quickly, and JIRA indeed has widespread usage.

Let's discuss the other two:

  • Rename both the content and IDL attribute. toplayer=""/topLayer comes to mind! Also popupbehavior=""/popupBehavior... consider getting creative with suffixes/prefixes/etc. I know bikeshedding sucks but it might just be the best solution in this case.

Ugh. But I think I agree this is likely the best option. But painful. Let the bikeshedding (re-)start.

  • Introduce an asymmetry between the content and IDL attribute. E.g. popup=""/htmlPopup, similar to for=""/htmlFor or value=""/defaultValue. Highly disrecommended; developers hate this and it's something we'd prefer to never do going forward. IMO this is strictly worse than (2).

I do realize this sucks for developers, but I think for this attribute, most of the usage will be via the content attribute. For example, in the <selectmenu> demo, which heavily uses the popup API, all 44 uses of popup are via the content attribute, or CSS selectors on the content attribute. Perhaps we should (re-)consider the popup=""/htmlPopup pattern?

@domenic
Copy link
Contributor

domenic commented Jun 13, 2022

I think it really depends on what context you're using it. For static demo sites, sure, use the content attribute. But e.g. everyone who would ever use this API through React, would use it via the property, because that's how JSX works there. (I.e. in React-JSX you do className=, not class=; similarly you'd do htmlPopup=, not popup=.)

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jun 14, 2022

I think it really depends on what context you're using it. For static demo sites, sure, use the content attribute. But e.g. everyone who would ever use this API through React, would use it via the property, because that's how JSX works there. (I.e. in React-JSX you do className=, not class=; similarly you'd do htmlPopup=, not popup=.)

It definitely depends on the context. In the React case, if you're using JSX (as I think/hope most uses would be), wouldn't the content attribute name be the one that matters? I.e. this JSX:

<div popup={myPopupType}> foo </div>

would get transformed to:

var reactNode = React.createElement('div', {popup: myPopupType}, "foo");

which should result (via this code, or this code for custom elements) in roughly this JS:

const div = document.createElement('div');
div.setAttribute('popup',myPopupType);

Of course, I could have this wrong, and that'd confirm your point about developer confusion.

If we go down the route of different content/IDL attribute names, I'd advocate for something that starts with "popup", so that autocomplete helps you. In general, here are some suggestions from a recent meeting (thanks @una, @jh3y, @argyleink):

  • myElement.popupAttr = 'auto';
  • myElement.attrPopup = 'auto';
  • myElement.popupValue = 'auto';
  • myElement.poopup = 'auto'; (Mostly a joke, but it's catchy.)

@domenic
Copy link
Contributor

domenic commented Jun 15, 2022

which should result (via this code, or this code for custom elements) in roughly this JS:

No, that's not my understanding. It would instead result in

const div = document.createElement('div');
div.popup =  myPopupType;

via this code since this code returns non-null because popup, as a standardized HTML attribute, will get added to the properties value in that file.

I'd continue to strongly suggest we never add new HTML attributes where the IDL and content attributes mismatch. I think that's an important property for future extensions to HTML.

@argyleink
Copy link

I'm not proud of this suggestion, but what about myElement.popUp = 'auto'; or myElement.Popup = 'auto';?

@domenic
Copy link
Contributor

domenic commented Jun 15, 2022

popUp could work. You would spell the feature "pop up" or "pop-up" in all documentation that wasn't about the attribute, similar to how you talk about an "enter key hint" for the enterkeyhint="" attribute, or the "image sizes" for the imagesizes="" attribute, or the "max length" for the maxlength="" attribute. Then it is natural that the IDL attribute is popUp.

I still think toplayer="" or popupbehavior="" are a bit better though.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jun 16, 2022

popUp could work. You would spell the feature "pop up" or "pop-up" in all documentation that wasn't about the attribute, similar to how you talk about an "enter key hint" for the enterkeyhint="" attribute, or the "image sizes" for the imagesizes="" attribute, or the "max length" for the maxlength="" attribute. Then it is natural that the IDL attribute is popUp.

Ohh I really like this suggestion. It nicely skirts the “content attribute not equal to IDL attribute” problem, and it’d be easy to change documentation to say “pop up” everywhere. I’d need to try it (or do the HTML Archive work) to see if it’s web compatible generally. But it at least fixes the one known breakage.

@dbaron
Copy link
Collaborator

dbaron commented Jun 16, 2022

FWIW, a suggestion (I think it was from flackr) which I liked, was the name popupType.

@dbaron
Copy link
Collaborator

dbaron commented Jun 16, 2022

#526 (comment) should have been in this issue

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jun 16, 2022

Per the comment above, these notes should have been here:

The Open UI Community Group just discussed https://github.com/openui/open-ui/issues/546, and agreed to the following:

  • RESOLVED: Rename the IDL attribute from 'popup' to 'popUp'. Also change documentation to use the phrase "pop-up" rather than "popup".
The full IRC log of that discussion
<JonathanNeal> topic: https://github.com/[/issues/526](https://github.com/openui/open-ui/issues/526)
<JonathanNeal> github: https://github.com/[/issues/526](https://github.com/openui/open-ui/issues/526)
<JonathanNeal> +1 to tantek’s suggestion
<dandclark> masonf: We talked about this a few weeks ago. This is the question about declarative triggering for hint popups.
<dandclark> masonf: Idea is you can put hint on any element. When you hover it, should trigger a hint. Like tooltip. Hover and it will show you tooltip about that thing.
<dandclark> masonf: Initial proposal was to use differerent declarative name. Discussed maybe it would be better to reuse showpopup, togglepopup.
<dandclark> masonf: My concern is there are use cases where that wouldn't work. Have button, want it to trigger popup, but also want a tooltip on that same element.
<dandclark> masonf: Wouldn't be possible if you combine them.
<dandclark> masonf: Also, what does hidepopup mean? Similarly togglepopup. For hints, doesn't make sense to hover the element, tooltip shows up, hover again, and it goes away. Triggering only goes in 1 direction.
<una> q+
<scotto_> q+
<dandclark> masonf: So there's general confusion I have and I wonder if we should revisit the earlier decision.
<JonathanNeal> q?
<JonathanNeal> ack una
<dandclark> una: You said it doesn't make sense to have hover toggle the tooltip. But we do want an action to hide the tooltip. Otherwise we don't be able to do things like animate tooltip out.
<dandclark> una: it's tricky but with tooltip I want to make sure it's not just an open action, and we also have close event that gets triggered.
<dandclark> masonf: Hint popup is still popup, has other popup behaviors includign show/hide events. And animation behaviors. So can animate based on :top-layer.
<dandclark> masonf: There is proposed behavior for how it goes away, modeled on tooltips. After CSS configrable delay after user mouses out, can go away. Can animate that.
<dandclark> una: There is use case you can create with this change
<dandclark> masonf: If we do all the things in this proposal and add another attriubte, can do all the things asked for.
<JonathanNeal> q?
<dandclark> scotto_: Even in demos I was making for sub navigation popup, trying to have that show up on hover, I found I had to reach for JS events to show/hide. Seems that having additional attr to help with showing hints on hover and focus would be beneficial to people who want to have popup subnavigation
<dandclark> scotto_: Hover it with mouse but want behavior to happen only on keyboard events.
<dandclark> scotto_: Could do that with JS, but do we want to make it available to people without JS
<dandclark> masonf: Tricky to do in JS. Handling of open/closed state.
<dandclark> masonf: Suggestion is to make this behavior is to make this work for non hint popups too?
<dandclark> scotto_: yes. There is use case for popup navigation. I don't necessarily like this pattern but it exists. People should be able to do this easily rather than futz with it in JS.
<JonathanNeal> q?
<JonathanNeal> ack scotto_
<dandclark> masonf: In your demo, you hover over something, menu shows up. Goes away when de-hovered?
<dandclark> scotto_: yes
<dandclark> masonf: That's hint popup
<dandclark> scotto_: It's not hint, it's subtnavigation.
<JonathanNeal> q+
<dandclark> masonf: Light dismiss desc for hint is different. It goes away when defocused. Also says you can't nest, but you have in this example. Interesting use case.
<dandclark> JohnathanNeal: Would this work for other popups where that behavior would be appropriate? e.g. CSS property that specifies these things, would that solve this use case?
<dandclark> masonf: You'd use non hint popup (auto), so you can nest it. Apply CSS property, control the fact that when dehovered, dismisses with CSS only. Could work.
<dandclark> JonathanNeal: Similar to transition delays.
<dandclark> masonf: Kind of , in that if you add this property with value you get new behavior. I'll think about that.
<dandclark> scotto_: Demo is in popup chat.
<masonf> q?
<JonathanNeal> ack JonathanNeal
<JonathanNeal> Up Next: Jonathan suggests `popup` itself becomes a CSS property. :P
<JonathanNeal> Kidding... For now...
<dandclark> masonf: Sounds like it needs more brainstorming.
<JonathanNeal> my-button { pop-up: auto; }
<dandclark> masonf: Undoing the last resolution? We need to use a different triggering attribute. Can't reuse others for this behavior, it's confusing and can't use both on same element.
<dandclark> scotto_: I agree with that, particularly for my use case. want to auto show on focus but not hover.
<masonf> Proposed resolution: use a new/different name for declarative hover-triggering of popups. Don't re-use togglepopup, showpopup, hidepopup.
<dandclark> masonf: even for hints, example is that you get different poups for hover vs for click.
<dandclark> s/poups/popups
<scotto_> +1 to proposed resolution
<chrisdholt> +1 to proposed
<tantek> +1
<JonathanNeal> Generally +1, but I don’t understand it well enough to feel authentic about it.
<JonathanNeal> q?
<masonf> RESOLVED: use a new/different name for declarative hover-triggering of popups. Don't re-use togglepopup, showpopup, hidepopup.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jun 16, 2022

Per the resolution, I'm going to close this issue, modify Chromium accordingly, and cross my fingers that we don't hit compat issues with popUp. If I do hit more, I'll open a fresh issue.

@mfreed7 mfreed7 closed this as completed Jun 16, 2022
@mfreed7 mfreed7 removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Jun 16, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 17, 2022
See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 21, 2022
See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 22, 2022
See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 23, 2022
See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
aarongable pushed a commit to chromium/chromium that referenced this issue Jun 23, 2022
See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703470
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017374}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 23, 2022
See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703470
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017374}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 23, 2022
See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703470
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017374}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 23, 2022
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 23, 2022
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 24, 2022
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
aarongable pushed a commit to chromium/chromium that referenced this issue Jun 24, 2022
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017676}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 24, 2022
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017676}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 24, 2022
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017676}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 29, 2022
…Up`, per resolution, a=testonly

Automatic update from web-platform-tests
Rename the `popup` IDL attribute to `popUp`, per resolution

See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703470
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017374}

--

wpt-commits: 350691e98b712351b5da4fc511a0f17a9cbec52e
wpt-pr: 34487
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 29, 2022
…p->hidePopUp, a=testonly

Automatic update from web-platform-tests
Rename showPopup->showPopUp and hidePopup->hidePopUp

Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017676}

--

wpt-commits: cd6f245502e992ef25dc2828f91f05a5f7dd689e
wpt-pr: 34570
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jun 30, 2022
…Up`, per resolution, a=testonly

Automatic update from web-platform-tests
Rename the `popup` IDL attribute to `popUp`, per resolution

See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703470
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017374}

--

wpt-commits: 350691e98b712351b5da4fc511a0f17a9cbec52e
wpt-pr: 34487
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jun 30, 2022
…p->hidePopUp, a=testonly

Automatic update from web-platform-tests
Rename showPopup->showPopUp and hidePopup->hidePopUp

Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017676}

--

wpt-commits: cd6f245502e992ef25dc2828f91f05a5f7dd689e
wpt-pr: 34570
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
See [1] for details, but `popup` as an IDL property is not web
compatible. The community has decided to try `popUp` to see if perhaps
that will be more compatible.

[1] openui/open-ui#546 (comment)

Bug: 1307772
Fixed: 1332480

Change-Id: I0b42c49a9c7b7fa7a6bf10c61443a8b52fb977bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703470
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017374}
NOKEYCHECK=True
GitOrigin-RevId: d2cd5fd0788216069f865c13507dcd3c7156839d
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Per the resolution [1] and associated discussion [2] we need to
rename showPopup to showPopUp, and hidePopup to hidePopUp. With
the new scheme, the API is the "Pop up API", not the "Popup API".

Note that this CL only changes the publicly-visible API from
"popup" to "popUp", but leaves many internal references such as
kPseudoPopupHidden instead of kPseudoPopUpHidden. crbug.com/1338587
tracks that follow-on work, which I'll do once this change is
validated as being web compatible.

[1] openui/open-ui#546 (comment)
[2] openui/open-ui#549

Bug: 1307772
Bug: 1338587
Change-Id: Iaa28d4c1b7d526bb0b06c04eaec2fad5d0431746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3717194
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017676}
NOKEYCHECK=True
GitOrigin-RevId: a561bcdeed3f502fb87db8c7c2bf134c0ae699d5
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 19, 2022
There was a previous web compat issue with event handler attributes
calling functions called "popup()". As a result, we made the `popup`
attribute unscopable, so this would not break the web. However,
with the change of name to `popUp` for the IDL attribute, this
compat issue likely doesn't exist, for the same reasons that this
rename fixed the general compat issue [1].

This CL removes the test for unscopable. The [2] CL actually made
the attribute scopable.

[1] openui/open-ui#546
[2] https://chromium-review.googlesource.com/c/chromium/src/+/3954386

Bug: 1307772
Change-Id: I603e75338e1ccb925e6f145870fa6fecc68c61fd
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 21, 2022
There was a previous web compat issue with event handler attributes
calling functions called "popup()". As a result, we made the `popup`
attribute unscopable, so this would not break the web. However,
with the change of name to `popUp` for the IDL attribute, this
compat issue likely doesn't exist, for the same reasons that this
rename fixed the general compat issue [1].

This CL removes the test for unscopable. The [2] CL actually made
the attribute scopable.

[1] openui/open-ui#546
[2] https://chromium-review.googlesource.com/c/chromium/src/+/3954386

Bug: 1307772
Change-Id: I603e75338e1ccb925e6f145870fa6fecc68c61fd
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 21, 2022
There was a previous web compat issue with event handler attributes
calling functions called "popup()". As a result, we made the `popup`
attribute unscopable, so this would not break the web. However,
with the change of name to `popUp` for the IDL attribute, this
compat issue likely doesn't exist, for the same reasons that this
rename fixed the general compat issue [1].

This CL removes the test for unscopable. The [2] CL actually made
the attribute scopable.

[1] openui/open-ui#546
[2] https://chromium-review.googlesource.com/c/chromium/src/+/3954386

Bug: 1307772
Change-Id: I603e75338e1ccb925e6f145870fa6fecc68c61fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3963911
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1062228}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 21, 2022
There was a previous web compat issue with event handler attributes
calling functions called "popup()". As a result, we made the `popup`
attribute unscopable, so this would not break the web. However,
with the change of name to `popUp` for the IDL attribute, this
compat issue likely doesn't exist, for the same reasons that this
rename fixed the general compat issue [1].

This CL removes the test for unscopable. The [2] CL actually made
the attribute scopable.

[1] openui/open-ui#546
[2] https://chromium-review.googlesource.com/c/chromium/src/+/3954386

Bug: 1307772
Change-Id: I603e75338e1ccb925e6f145870fa6fecc68c61fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3963911
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1062228}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 21, 2022
There was a previous web compat issue with event handler attributes
calling functions called "popup()". As a result, we made the `popup`
attribute unscopable, so this would not break the web. However,
with the change of name to `popUp` for the IDL attribute, this
compat issue likely doesn't exist, for the same reasons that this
rename fixed the general compat issue [1].

This CL removes the test for unscopable. The [2] CL actually made
the attribute scopable.

[1] openui/open-ui#546
[2] https://chromium-review.googlesource.com/c/chromium/src/+/3954386

Bug: 1307772
Change-Id: I603e75338e1ccb925e6f145870fa6fecc68c61fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3963911
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1062228}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 7, 2022
…` attr, a=testonly

Automatic update from web-platform-tests
Remove pop-up test for unscopable `popup` attr

There was a previous web compat issue with event handler attributes
calling functions called "popup()". As a result, we made the `popup`
attribute unscopable, so this would not break the web. However,
with the change of name to `popUp` for the IDL attribute, this
compat issue likely doesn't exist, for the same reasons that this
rename fixed the general compat issue [1].

This CL removes the test for unscopable. The [2] CL actually made
the attribute scopable.

[1] openui/open-ui#546
[2] https://chromium-review.googlesource.com/c/chromium/src/+/3954386

Bug: 1307772
Change-Id: I603e75338e1ccb925e6f145870fa6fecc68c61fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3963911
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1062228}

--

wpt-commits: e26f46e26e4b94d890b34e9a73b58f5a395ff2b8
wpt-pr: 36545
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 11, 2022
…` attr, a=testonly

Automatic update from web-platform-tests
Remove pop-up test for unscopable `popup` attr

There was a previous web compat issue with event handler attributes
calling functions called "popup()". As a result, we made the `popup`
attribute unscopable, so this would not break the web. However,
with the change of name to `popUp` for the IDL attribute, this
compat issue likely doesn't exist, for the same reasons that this
rename fixed the general compat issue [1].

This CL removes the test for unscopable. The [2] CL actually made
the attribute scopable.

[1] openui/open-ui#546
[2] https://chromium-review.googlesource.com/c/chromium/src/+/3954386

Bug: 1307772
Change-Id: I603e75338e1ccb925e6f145870fa6fecc68c61fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3963911
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1062228}

--

wpt-commits: e26f46e26e4b94d890b34e9a73b58f5a395ff2b8
wpt-pr: 36545
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API
Projects
None yet
Development

No branches or pull requests

4 participants