Skip to content

Improve embedded form support in iframes#3010

Open
k4sperski wants to merge 6 commits intoAutomattic:masterfrom
k4sperski:issue/handle-embedded-forms
Open

Improve embedded form support in iframes#3010
k4sperski wants to merge 6 commits intoAutomattic:masterfrom
k4sperski:issue/handle-embedded-forms

Conversation

@k4sperski
Copy link
Contributor

@k4sperski k4sperski commented Mar 23, 2026

Issues

Related: Discussion #2990

Depends on: PR #2998 for the full end-to-end behaviour on pages that replace the body/DOM after load.

Description

User Impact

This PR improves Harper's behaviour in embedded third-party forms in the chrome extension.

Before this patch:

  • Harper could be enabled on the top-level page
  • the embedded iframe would still not lint unless its own host was explicitly enabled
  • even when the iframe was eligible, a textarea revealed after user interaction could still be missed by the initial scan

After this patch:

  • an enabled parent page can allow linting in an unset embedded iframe
  • popup-style www. keys continue to work for parent-origin fallback
  • explicit iframe overrides still take precedence over inherited parent state
  • a dynamically revealed iframe textarea is attached when the user focuses it

Technical Details

This PR makes two focused changes that are needed for the end-to-end embedded-form flow:

  1. Parent-origin inheritance for iframe lint requests:
    If an iframe host does not have an explicit stored setting, Harper now falls back to the top-level tab host and allows linting when that parent domain is enabled.

    This remains a narrow lookup change:

    • inheritance only applies when the iframe host is unset
    • explicit iframe opt-in still wins when the parent is disabled
    • explicit iframe opt-out still wins when the parent is enabled
    • this does not introduce suffix matching, registrable-domain matching, or a broader domain inheritance model
  2. Focus-based attachment for lazily revealed textareas:
    Some embedded form providers only insert or reveal the actual textarea after the user interacts with the form. To cover that flow, the content script now also attaches to eligible textareas (HTMLTextAreaElement) when they receive focus.

I also aligned the background read path with the popup's existing www. behaviour. When the parent page is www.example.com, background lookups now also check the popup-style example.com key during reads.

Demo

One concrete example is the embedded Greenhouse form on emnify page.

In this example part of the manual-entry flow is revealed only after user clicks Enter manually causing textarea to appear after Harper's normal initial scan window, which is why the focus-based attachment fallback matters here.

How Has This Been Tested?

I verified this locally with extension builds and Playwright coverage. The new iframe inheritance coverage includes:

  • parent enabled, child unset
  • parent enabled through a www. parent host when the stored key follows the popup's existing www.-stripped pattern
  • parent enabled, child explicitly disabled
  • parent disabled, child unset
  • parent disabled, child explicitly enabled

The dynamic textarea regression verifies that Harper still attaches after the textarea is inserted or revealed after the normal initial scan window.

Firefox notes:

  • manually verified on the emnify/Greenhouse flow when combined with PR #2998
  • the new iframe Playwright specs work with Chrome only because Firefox extension-state seeding is not reliable in the current test harness

Checklist

  • I have performed a self-review of my own code
  • I have added tests to cover my changes

@k4sperski
Copy link
Contributor Author

One thing I want to flag explicitly is the scope of the new focusin fallback. The bug here is that some embedded forms reveal the real HTMLTextAreaElement only after interaction, so scan-based attachment misses it. This fixes that, but the current implementation applies to focused HTMLTextAreaElements generally, not only iframe specific flows. I think that is a reasonable trade-off, but it feels like a maintainer call on scope rather than a strict correctness issue. I’m happy to adjust it!

@k4sperski
Copy link
Contributor Author

k4sperski commented Mar 23, 2026

Another good example found here on applications form on careers page

k4sperski:issue/handle-embedded-forms
Screenshot 2026-03-23 at 17 46 34

Automattic:master
Screenshot 2026-03-23 at 17 47 03

@elijah-potter
Copy link
Collaborator

Awesome! I wonder if this will also address #2077?

Copy link
Collaborator

@elijah-potter elijah-potter left a comment

Choose a reason for hiding this comment

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

This looks great! Overall, your work is quite legible, but I'd like to see some more explicit documentation. Just a few tweaks and this will be ready to merge.

As for the callback, it's a small enough change that I don't think it really matters if its in scope. It doesn't meaningfully adjust the amount of effort it takes to review the PR. Keep it.

@k4sperski
Copy link
Contributor Author

k4sperski commented Mar 24, 2026

Hi @elijah-potter! Thank you for the review. By “scope,” I meant that the new fallback focusin behaviour applies to all HTMLTextAreaElements, not just those within the iframe context where the issue was originally discovered. That is not necessarily a bad thing, since it casts a wider net for textareas the initial scans may have missed, but I wanted to call it out in case introducing such broader behaviour is not desired.

I'll take a closer look at #2077 and see if this is still an issue that needs addressing.
^ never mind, if I am not mistaken, this tool is not something I can easily get my hands on outside of the edu system...

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants