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

popover attribute may not be web compatible #9042

Closed
nt1m opened this issue Mar 17, 2023 · 16 comments
Closed

popover attribute may not be web compatible #9042

nt1m opened this issue Mar 17, 2023 · 16 comments
Labels
compat Standard is not web compatible or proprietary feature needs standardizing topic: popover The popover attribute and friends

Comments

@nt1m
Copy link
Member

nt1m commented Mar 17, 2023

We just received a bug report that https://lrn.com/ is applying the popover attribute on some elements on their training pages which causes them to disappear on engines that implement the popover attribute and prevents from moving forward in the training process.

@mfreed7 @josepharhar @miketaylr Does Chrome have some use counters to measure potential breakage in the wild? I'm curious if Chrome has a plan to mitigate for such breakage.

@nt1m
Copy link
Member Author

nt1m commented Mar 17, 2023

(An avenue that's always possible for WebKit is to quirk specific sites to disable the popover attribute UA styles, but it's not ideal)

@nt1m nt1m added the topic: popover The popover attribute and friends label Mar 17, 2023
@nt1m
Copy link
Member Author

nt1m commented Mar 17, 2023

cc @ziransun @asurkov @cathiechen @emilio who are involved in the Gecko implementation for awareness.

@nt1m
Copy link
Member Author

nt1m commented Mar 17, 2023

(fwiw I don't have a direct link to the page that's broken since you need company logins to access it, but the gist of it is that it's broken because elements with the popover attribute now have display: none by default).

@miketaylr
Copy link
Member

Heya @nt1m - we do have use counters at https://chromestatus.com/metrics/feature/popularity#PopoverTypeAuto (and see the two below it. Usage is basically zero... but maybe the UseCounters are only active for folks with chrome://flags/#enable-experimental-web-platform-features on? (I dunno, maybe @mfreed7 or @josepharhar would know).

I'm happy to attempt outreach, but would need more details about the report - feel free to shoot me an email if that would be helpful.

@mfreed7
Copy link
Contributor

mfreed7 commented Mar 17, 2023

Thanks for raising this! I've had popover enabled at 50% of Canary/Dev/Beta versions of Chrome for the last 3-4+ months, to suss these out. There was one other issue like this, and after some quick outreach, the site fixed their bug. Other than that, I haven't seen any other bugs, so I'm fairly confident this is a low frequency issue.

To be very clear, this is a site bug. Custom attributes must start with data- to avoid clashes with platform features like this. An easy recommended fix for the site is to just rename popover to data-popover.

Does anyone have a good contact at the broken site? I'm happy to try to reach out to help them.

@nt1m
Copy link
Member Author

nt1m commented Mar 17, 2023

Thanks for the insight! I'm currently attempting outreach to get this site fixed, will keep you updated when I get more info.

@mfreed7
Copy link
Contributor

mfreed7 commented Mar 17, 2023

Thanks for the insight! I'm currently attempting outreach to get this site fixed, will keep you updated when I get more info.

Awesome, thanks! LMK if you need any help.

@annevk annevk added the compat Standard is not web compatible or proprietary feature needs standardizing label Mar 22, 2023
@nt1m
Copy link
Member Author

nt1m commented Mar 29, 2023

Small update here: outreach is still in progress. Seems like the framework that was used on the site was an older version of Angular UI Bootstrap: https://angular-ui.github.io/bootstrap/ (from @karlcow's analysis), which used the popover attribute.

@nt1m
Copy link
Member Author

nt1m commented Apr 13, 2023

Fwiw, we've had a second report from a mobile app using the ionic framework which incorrectly uses the popover attribute: https://ionicframework.com/docs/api/popover

@mfreed7
Copy link
Contributor

mfreed7 commented Apr 13, 2023

Fwiw, we've had a second report from a mobile app using the ionic framework which incorrectly uses the popover attribute: https://ionicframework.com/docs/api/popover

Thanks for the heads up. Is there a bug filed against ionic for this already?

@nt1m
Copy link
Member Author

nt1m commented Apr 17, 2023

It does look like this was fixed in ionic-team/ionic-framework#26672, though some websites/apps might still use the older version of the framework.

@nt1m
Copy link
Member Author

nt1m commented May 3, 2023

We found out that sites using older versions of this framework are affected: valor-software/ngx-bootstrap#6544

@Totati
Copy link

Totati commented May 26, 2023

I've found another one: AlaskaAirlines/auro-popover#42

@mfreed7
Copy link
Contributor

mfreed7 commented May 26, 2023

I've found another one: AlaskaAirlines/auro-popover#42

Thanks! I just commented there. Hopefully they can fix it soon.

@nt1m
Copy link
Member Author

nt1m commented May 26, 2023

We found out that sites using older versions of this framework are affected: valor-software/ngx-bootstrap#6544

Correction: no fix was ever done in the framework itself, but framework consumers are encouraged to move from popover= to [popover]= to avoid the clash.

@mfreed7
Copy link
Contributor

mfreed7 commented Jul 13, 2023

I think we can close this issue. Chrome shipped Popover in M114, and while there were a few issues (including the ones mentioned above), they were all resolved on the site side by avoiding non-data--prefixed attributes.

@mfreed7 mfreed7 closed this as completed Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat Standard is not web compatible or proprietary feature needs standardizing topic: popover The popover attribute and friends
Development

No branches or pull requests

5 participants