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

(dev/core#2994) IFrame Connector - Add extension to allow remote iframes (*incubation*) #29496

Merged
merged 30 commits into from
Mar 1, 2024

Conversation

totten
Copy link
Member

@totten totten commented Feb 24, 2024

Overview

The humble <iframe> has been around since HTML 4, and it provides a widely-known mechanism to embed content from other web-sites. However, modern <iframe>s are more complicated than their ancestors -- they require additional adaptations to operate with current browsers. And (at the CMS layer) <iframe> support is often disabled by default.

This PR provides opt-in support for <iframe>s for CiviCRM. By enabling the extension ("IFrame Connector"), one can allow external web-sites to embed CiviCRM pages.

  • This is a key building-block for oEmbed support (https://lab.civicrm.org/dev/core/-/issues/2994). oEmbed providers often use IFRAMEs.
  • The extension should be regarded as incubating. The PR has the main concepts. Both @seamuslee001 and I have been able to use it. It has connectors for UF=Drupal and UF=Drupal8. But we need to develop similar connectors for other UFs, and we will probably revise the options further in the coming months. Therefore, it is hidden (<tag>mgmt:hidden</tag>) from the main list of extensions.
  • This is a re-roll of Oembed Support #29075. It's rebased. Debug comments have been removed. The README and info.xml have been cleaned up more. The extension has been renamed(oembed=>iframe) from earlier drafts.

Before

  • In ancient times, you could kind-of put CiviCRM content in an IFRAME.
  • In modern times, it's not very reliable. There are traps.

After

  • You can enable the "IFrame Connector" extension. And then certain pages (e.g. public-facing pages) can be embedded.

Quick Start

  • Enable the extension via CLI/API:

    cv en iframe
  • The "System Status" will prompt you to deploy the connector script:

    Screenshot 2024-02-23 at 8 52 50 PM
  • In "Administer > System Settings > IFrame Connector", you can configure how embedded pages will work:

    Screenshot 2024-02-23 at 8 44 42 PM
  • Pick a public CiviCRM page (eg civicrm/contribute/transact?reset=1&id=1).

  • On an external website, you can make an HTML page which embeds the CiviCRM page:

    <IFRAME SRC="http://example.org/iframe.php/civicrm/contribute/transact?reset=1&id=1"/>

Comments

The included ext/iframe/README.md includes more, discussion, and list of TODOs.

Why rename the extension? In retrospect, the current code is actually about IFRAMEs. IFRAMEs can be used with or without oEmbed. The follow-up will provide an oembed extension which is actually about oEmbed (i.e. the protocol for translating copy-pasted URLs into IFRAME snippets). And oembed will depend on iframe.

__Background__: The `Civi::url()` helper takes logical URLs
(`frontend://civicrm/event/info`) and translates to physical URLs.

__Before__: The schemes are specifically `frontend://`, `backend://`, and `current://`.
Logical schemes are only defined by `civicrm-core` and its UF-integrations.

__After__:

* Extensions can define new logical schemes.
* The `http://` and `https://` schemes are also supported.

__Technical Details__:

* To add a new scheme, you implement event `civi.url.render.MYSCHEME`.
  The listener should translate the logical URL into a physical URL.
* Since physical URLs will typically be HTTP(S), we add some more
  helpers that are convenient for outputing HTTP(S), as in:
  In particular, the `merge()` utility should help with this.
Copy link

civibot bot commented Feb 24, 2024

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...
  • If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
  • A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
  • A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
  • If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
  • Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
  • A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
  • To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
  • For more information about contributing, see CONTRIBUTING.md.
Quick links for reviewers...

➡️ Online demo of this PR 🔗

@civibot civibot bot added the master label Feb 24, 2024
totten and others added 21 commits February 25, 2024 17:45
The `System::url()` API has a contract designed for a "two scheme world"
(frontend or backend).  The gist of the contract is this:

- By default, the URL is rendered in the *current* environment (frontend xor backend)
- You may optionally request a specific environment (`$frontend=TRUE` or `$forceBackend=TRUE`).

We're entering a "multi-scheme" world (with frontend or backend; and also:
web-service or oembed).  This is more apparent in the `Civi::url()` API,
where you can give a scheme (`current://` or `frontend://` or `oembed://`).
But we still have many calls to `System::url()`.

The patch preserves the behavior of `System::url()` for all normal work in the existing
frontend/backend environments. But it adds support for other "current" environments:

==> If you're processing an oembed page-view and call `System::url()` (with
    default options), then it should return the oembed-style URL.

Of course, if you give an explicit flag (`$frontend`, `$forceBackend`), then that
will still be respected.
@totten
Copy link
Member Author

totten commented Feb 26, 2024

There's currently one test failure (api\v4\Action\ContactGetTest.testAge). I don't think this is related. At least, it's hard to see a connection, and the same error appears on a different/concurrent PR (build 9400/PR 29496/test-3 vs build 9401/PR 29504/test-4).

@seamuslee001
Copy link
Contributor

Yes I can confirm I have been able to generate a suitable iframe where d10 is the provider and a WP is consumer so far

.gitignore Outdated
@@ -22,6 +22,7 @@
!/ext/financialacls
!/ext/contributioncancelactions
!/ext/recaptcha
!/ext/oembed
Copy link
Contributor

Choose a reason for hiding this comment

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

@totten it seems you have re-named the ext to be iframe but this is still set to be oembed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @seamuslee001. Fixed.

@colemanw
Copy link
Member

colemanw commented Mar 1, 2024

Reviewed the code, tried out the functionality. Good to merge.

@totten
Copy link
Member Author

totten commented Mar 1, 2024

Failures are a Leap Day Present.

@totten totten merged commit ddf419b into civicrm:master Mar 1, 2024
2 of 3 checks passed
@totten totten deleted the master-iframe branch March 1, 2024 03:40
@demeritcowboy
Copy link
Contributor

Every page in drupal 8 is now giving php warnings - followup PR: #29560

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants