jgerigmeyer
commented
6 months ago @jamesnw Could you look into this?
Closed cwoolum closed 5 months ago
jgerigmeyer
commented
6 months ago @jamesnw Could you look into this?
jamesnw
commented
6 months ago @cwoolum Interesting- I'm not able to replicate in Chrome 110. This is what I'm seeing-
The isSupported logic isn't complex, and hopefully we can get more info there.
What is the result of 'popover' in HTMLElement.prototype in your Dev Tools Console in Chrome 110, ideally on chrome://version/?
Do you have any browser extensions that could be impacting this? Any other ideas on how to replicate?
cwoolum
commented
6 months ago I'm sorry, I just realized I used Chrome all over the place when I meant Chromium.
These are all clean unzips(not even installed) from the artifact repos so there are no profiles or extensions. Checking if popover exists on prototype does return true for me.
jamesnw
commented
6 months ago Can you confirm that you have the Experimental Web Platform Features feature flag disabled? In Chromium 110, enabling that enables popover. If that's disabled, do you have any other feature flags enabled?
Also, it looks like you have a long list of Active Variations- those often are related to feature flags, or if you are launching Chromium from the command line, controlled with --enable-features and --disable-features. Are any of those enabling Popover?
cwoolum
commented
6 months ago I'm not launching Chromium with any special features. For all of this testing, I'm just clicking on Chrome.exe in the unzipped folder. I didn't find any flags related to popover specifically.
It looks like experimental web platform features are disbled.
Are you not able to replicate the issue by using one of the zips above?
jamesnw
commented
6 months ago I'm on Mac, so I wasn't able to confirm with those downloads. I was able to find an old Mac version from the Chromium snapshots and replicate what you're seeing.
What I'm guessing is that Chromium version 110 had partial experimental support for popover, hidden behind a feature flag, but different enough from the final spec that popover written to the current spec do not work. For some reason, the Chromium snapshots that you're using have this enabled, (and not through a feature flag you can disable).
In other words, you're testing what a user would see if they had Chromium version 110 with the experimental popover enabled (which is possible, but unlikely). My recommendation would be for you to find a way to disable that variation (or find a Chromium download without that enabled), so you can test what a more likely user would see.
I wasn't able to find a way to disable it, but if you are able to, please report back!
cwoolum
commented
6 months ago I wasn't able to find a way to disable the flag either. It looks like 108 is pretty similar in terms of other functionality so I'll probably just go with that. It might be good to just have a note staying that Chromium 109-113 are broken although, like you said, it's very unlikely someone would be using those versions.
jamesnw
commented
6 months ago I've opened a PR to add a general caveat to the readme- thanks for reporting back!
mfreed7
commented
6 months ago Just to quickly chime in here, this is a bit odd. I'm not sure what's going on, since you're correct that the feature was enabled by default in M114, and before that the popover attribute should not have been included without the feature being enabled. I did do a bit of experimentation via Finch on pre-stable releases, to make sure there weren't compat problems with the popover attribute name, as there were with popup. But I don't think those should have affected the Chromium downloads linked above.
I bisected the addition of popover to the HTMLElement prototype, and it happened somewhere in this range. I definitely made a bunch of popover related changes in that range, but none of them that I can see should have done that! Let me know if you see something there that I missed.
TL;DR: sorry. Hopefully it works to test with 108?
jamesnw
commented
6 months ago @mfreed7 Thanks for taking a look. As best as I can understand at this point-
showPopover() works without the feature flag turned onchrome:versions page, but don't know how to match the hash to a feature, or how to control which ones are enabledpopover on the prototypeSo it isn't that 'popover' in HTMLElement.prototype is true but popover isn't enabled, the issue is that popover is enabled (so the polyfill can't be applied), but it doesn't work according to the latest spec.
mfreed7
commented
6 months ago So it isn't that
'popover' in HTMLElement.prototypeis true but popover isn't enabled, the issue is that popover is enabled (so the polyfill can't be applied), but it doesn't work according to the latest spec.
Right thanks, I understood that. I'm just not sure how it's getting enabled in pre-114 versions. If you want to attach the variations from e.g. 112, I can look them up and see if there's something popover-related there.
jamesnw
commented
6 months ago Sure, here's a list of the Active variations. I assume the list of variations that were active from 109 to 113 will be more useful, but it just narrows the haystack down to 100 variations.
mfreed7
commented
6 months ago Yep! That's it. 263848e4-3f4a17df (present in all) is Popover-enabled. I guess the Chromium builds you are using are on the Dev channel and get Finch updates. And those get the experiment I ran for compatibility of popover as an attribute.
Sorry for the trouble. But I'm very confident that production stable builds of Chrome did not have the attribute enabled prior to 114. You can test by explicitly passing --disable-blink-features=HTMLPopoverAttribute on the command line, which will override any Finch config.
Describe the bug When using older versions of chromium that didn't support popover, the polyfill incorrectly assumes that popover is natively supported and the functionality is broken.
To Reproduce Steps to reproduce the behavior:
I used Windows in all cases.
Expected behavior I would expect the polyfill to work in versions 109-113 of chromium
Screenshots Here is a screenshot of the issue on Chrome 110.
Browsers tested on Windows, Chrome 104-114, Desktop X64
Additional context We are trying to use Chrome 113 for backwards compatibility testing of our library. We could go back to an older version of chromium but the newest version that doesn't support popover would be ideal.