fix(popover): popover opens on chrome 109

[liamdebeasi]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Some docs updates need to be made in the ionic-docs repo, in a separate PR. See the contributing guide for details.
• Build (npm run build) was run locally and any changes were pushed
• Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

Issue URL: resolves #26643

Popover currently passes an unused popover property to the component. However, popover is not a property on ion-popover. This was added in 6383a54

This interferes with the new Popover API in Google Chrome and causes the popover to not appear.

What is the new behavior?

• Remove the popover and modal properties from being passed in core

I updated the Angular Delegate to pass these properties directly to the Angular component for backwards compatibility. This API is a legacy behavior and is not well documented. This API was never usable in Ionic React and Ionic Vue. TypeScript would complain about accessing a property that does not exist on the component props interface, and we explicitly removed these keys from the componentProps in Ionic Vue. We should explore alternative APIs in the future.

Does this introduce a breaking change?

• Yes
• No

Other information

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[sean-perkins]

It is unfortunately that Google added a new reserved property to the HTMLElement. I know for modal we do something similar and that the modal property was a required part of an implementation reported to us (cannot recall the issue).

Instead of removing this information from the user element, should we instead rename popover → overlay or popover → popoverEl?

[liamdebeasi]

I found a way to keep backwards compatibility while fixing the issue. Can you take another look?

[sean-perkins]

Syncing some of our conversation into this thread.

I am a little confused why Angular has this problem to begin with and if any change is needed to the Angular delegate after all.

The root issue is that Chrome 109 adds a new property to the HTMLElement that we also assign to.

However, Angular encapsulates a component into an instance and the element reference (HTMLElement). We are assigning the component properties (the reserved popover property) to the component instance: https://github.com/ionic-team/ionic-framework/blob/main/angular/src/providers/angular-delegate.ts#L131 not the HTMLElement itself.

This is different than the core implementation, which writes directly to the HTMLElement: https://github.com/ionic-team/ionic-framework/blob/main/core/src/utils/framework-delegate.ts#L28

With the original issue, they observed it in our docs (which uses the core delegate). I'm unclear based on the issue which Stackblitz flavor they used (Angular is selected, but I was not able to reproduce).

Am I missing something?

[liamdebeasi]

The offending core code (https://github.com/ionic-team/ionic-framework/pull/26672/files#diff-a9009029fec1745d56894683444eca4f67bd76cd37b4d151471579f7efe26fa8L451-L455) was added so Angular developers can access an instance of popover or modal from their component code. This code is triggering the popover warning in Chrome 109.

I moved the logic from core to the Angular delegate so that Angular developers still have access to this functionality but in a way that does not trigger the warning (since we are assigning the properties to the component instance as you noted).

This PR doesn't fix the bug in Angular, it adds code to Angular for backwards compat so we can safely remove the code from Core that is causing the bug.

[sean-perkins]

Oh I see... I was conflating the two PRs. This is moving the assignment for the modal and popover properties into the framework delegate itself. The other PR is going to remove the property from being set/passed through the params argument of attachViewToDom.

Ok this makes sense 👍

[IAM5K]

Hopefully modifications may fix the issue.

[benoitbzl]

Hello,
is there any chance that this fix is backported to ionic 5? We are facing this issue, and we cannot update today our mobile application to ionic 6 or 7.

FYI, the examples in the documentation of the popover service and ion-select v5 are currently broken.

[kavishsoam1]

is there any solution in ionic 5 for popover, in case someone is not looking to update his ionic version?

[liamdebeasi]

Hi everyone,

We provided a workaround for Ionic v4 and v5 apps on #27599 (comment).

[benoitbzl]

Thanks @liamdebeasi for the link. Strangely I am not facing the issue for popovers created with the popover controller (I am using Ionic Angular v5). But I have the issue for ion-select components when using the popover interface. Is there a workaround for that case?

[liamdebeasi]

You could try calling removeAttribute on the popover when ionPopoverWillPresent fires. You'll want to add the listener on the document/window since the popover is mounted as a child of ion-app not ion-select.

[kenwalker]

Thanks @liamdebeasi for the link. Strangely I am not facing the issue for popovers created with the popover controller (I am using Ionic Angular v5). But I have the issue for ion-select components when using the popover interface. Is there a workaround for that case?

I have the same issue. I have HTML that worked fine on Android and on Chrome on Ionic Angular v5 but now both throw the "Found a popover" warning and the selection values do not show up.

<ion-select [(ngModel)]="level" name="level" value="1" interface="popover">

Trying to remove that attribute before the warning/error occurs in ionPopoverWillPresent seems too late. It comes in after the error has happened already.