Skip to content

Allow iframe links to open on android through a parameter#15063

Merged
bramkragten merged 6 commits into
home-assistant:devfrom
cnico:dev
Jan 24, 2023
Merged

Allow iframe links to open on android through a parameter#15063
bramkragten merged 6 commits into
home-assistant:devfrom
cnico:dev

Conversation

@cnico
Copy link
Copy Markdown
Contributor

@cnico cnico commented Jan 9, 2023

Breaking change

None

Proposed change

Following the discussion in home-assistant/android#3044 and my previous PR in #14379, I propose the approach of adding the 'allow-top-navigation-by-user-activation' value for the sandbox's iframe attribute.
By default, it is not allowed and if the user chooses it, it is allowed.

So it resolves my initial problem of not being able to clic on the links of the iframe in my android device.
I tested it in developpement mode and it works well.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Create an iframe card in lovelace with the following src url : https://www.programme-television.org/?no_layout=
Check or un check the new option and see in debug mode, the presence or absence of the sandbox="allow-top-navigation-by-user-activation" value.
Open it on your android device and clic on a link of the tv show : it opens correctly now in a tab of the default android browser.

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

alanmilinovic
alanmilinovic previously approved these changes Jan 17, 2023
Copy link
Copy Markdown

@alanmilinovic alanmilinovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the change and it is working fine.

@alanmilinovic
Copy link
Copy Markdown

Wow how much time is needed for a simple code review.

@bramkragten
Copy link
Copy Markdown
Member

Wow how much time is needed for a simple code review.

It is not perse the code review, more if we want to add this feature...

Comment thread src/panels/lovelace/cards/hui-iframe-card.ts Outdated
Comment thread src/panels/lovelace/editor/config-elements/hui-iframe-card-editor.ts Outdated
Comment thread src/translations/en.json Outdated
Comment thread src/panels/lovelace/editor/config-elements/hui-iframe-card-editor.ts Outdated
@bramkragten
Copy link
Copy Markdown
Member

Please add a documentation PR for this option

@cnico
Copy link
Copy Markdown
Contributor Author

cnico commented Jan 23, 2023

@bramkragten , I did not know we could set options via the code editor but not via the UI.
I removed the UI config options as you suggest.
I also synced my fork to the upstream and tested it. It works via configuration in the code editor.

@cnico
Copy link
Copy Markdown
Contributor Author

cnico commented Jan 23, 2023

And finaly added the documentation PR also.

@bramkragten bramkragten enabled auto-merge (squash) January 24, 2023 13:11
@bramkragten bramkragten merged commit 8935dba into home-assistant:dev Jan 24, 2023
@alanmilinovic
Copy link
Copy Markdown

Will this change be in a next release? Thank you.

@github-actions github-actions Bot locked and limited conversation to collaborators Jan 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants