Skip to content

GH#1240: fix: correct iframe body lookup for iOS adjustments#1245

Merged
superdav42 merged 1 commit into
mainfrom
feature/auto-20260521-024929-gh1240
May 21, 2026
Merged

GH#1240: fix: correct iframe body lookup for iOS adjustments#1245
superdav42 merged 1 commit into
mainfrom
feature/auto-20260521-024929-gh1240

Conversation

@superdav42
Copy link
Copy Markdown
Collaborator

@superdav42 superdav42 commented May 21, 2026

Summary

Implementation for issue #1240.

Files Changed

assets/js/template-previewer.js

Runtime Testing

  • Risk level: Low (agent prompts / infrastructure scripts)
  • Verification: shellcheck clean, self-assessed

Resolves #1240


aidevops.sh v3.17.18 plugin for OpenCode v1.15.6 with claude-haiku-4-5 spent 50s and 2,672 tokens on this as a headless worker.

Summary by CodeRabbit

  • Bug Fixes
    • Improved iOS Safari preview functionality by enhancing the loading indicator handling and refining iframe detection for a more reliable preview experience on Safari.

Review Change Stack

@superdav42 superdav42 added the origin:worker Auto-created by pulse labelless backfill (t2112) label May 21, 2026
@superdav42
Copy link
Copy Markdown
Collaborator Author

Completion Summary


aidevops.sh v3.17.18 plugin for OpenCode v1.15.6 with claude-haiku-4-5 spent 57s and 2,672 tokens on this as a headless worker.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

📝 Walkthrough

Walkthrough

The DOMContentLoaded iframe load callback now hides the loading indicator and uses safer contentDocument-based body access for iOS Safari preview adjustments instead of searching the iframe element tree.

Changes

iOS Safari Preview Iframe Fix

Layer / File(s) Summary
Iframe body lookup and loading indicator
assets/js/template-previewer.js
The iframe body is now accessed via iframeEl.contentDocument.body instead of getElementsByTagName, and wu-loading-indicator is hidden when present, ensuring the iOS Safari preview class and style patches target the correct document body.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A fix for the body, so clever and neat,
contentDocument access makes iOS complete!
No more false searches on element trees,
The iframe shines bright now, the rabbit agrees

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: correcting the iframe body lookup for iOS adjustments, which is the core objective of this PR.
Linked Issues check ✅ Passed The PR implements the suggested fix from issue #1240: it corrects the iframe body lookup by using iframeEl.contentDocument.body instead of getElementsByTagName, and adds the wu-loading-indicator hiding step.
Out of Scope Changes check ✅ Passed All changes are directly related to the iOS iframe body lookup fix specified in issue #1240; no out-of-scope modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/auto-20260521-024929-gh1240

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

🔨 Build Complete - Ready for Testing!

📦 Download Build Artifact (Recommended)

Download the zip build, upload to WordPress and test:

🌐 Test in WordPress Playground (Very Experimental)

Click the link below to instantly test this PR in your browser - no installation needed!
Playground support for multisite is very limitied, hopefully it will get better in the future.

🚀 Launch in Playground

Login credentials: admin / password

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@assets/js/template-previewer.js`:
- Around line 77-78: The iframe body lookup currently only uses
iframeEl.contentDocument and can miss the document on some platforms (e.g. iOS);
update the lookup to fallback to iframeEl.contentWindow?.document and then grab
.body from that document, reusing the existing iframe reference (the previously
created iframe variable) instead of re-querying the DOM; ensure the logic sets
body = (iframeEl.contentDocument || iframeEl.contentWindow?.document)?.body so
the iOS patch runs reliably.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d53dd648-964d-44a0-8e81-c6f0f52ceae6

📥 Commits

Reviewing files that changed from the base of the PR and between dcdb8da and 5d1c46e.

📒 Files selected for processing (1)
  • assets/js/template-previewer.js

Comment on lines +77 to +78
const iframeEl = document.getElementById("iframe");
const body = iframeEl && iframeEl.contentDocument ? iframeEl.contentDocument.body : null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add iframe document fallback to avoid silently skipping the iOS patch.

At Line 78, using only contentDocument can still miss the iframe body in some cases. Use contentWindow?.document as fallback (and reuse the existing iframe reference from Line 61).

Suggested patch
-				const iframeEl = document.getElementById("iframe");
-				const body = iframeEl && iframeEl.contentDocument ? iframeEl.contentDocument.body : null;
+				const iframe_doc = (iframe == null ? void 0 : iframe.contentDocument) || (iframe == null ? void 0 : iframe.contentWindow == null ? void 0 : iframe.contentWindow.document);
+				const body = iframe_doc ? iframe_doc.body : null;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@assets/js/template-previewer.js` around lines 77 - 78, The iframe body lookup
currently only uses iframeEl.contentDocument and can miss the document on some
platforms (e.g. iOS); update the lookup to fallback to
iframeEl.contentWindow?.document and then grab .body from that document, reusing
the existing iframe reference (the previously created iframe variable) instead
of re-querying the DOM; ensure the logic sets body = (iframeEl.contentDocument
|| iframeEl.contentWindow?.document)?.body so the iOS patch runs reliably.

@superdav42 superdav42 merged commit f18968f into main May 21, 2026
11 checks passed
@superdav42
Copy link
Copy Markdown
Collaborator Author

Completion Summary


aidevops.sh v3.17.18 plugin for OpenCode v1.15.6 with claude-haiku-4-5 spent 57s and 2,672 tokens on this as a headless worker.


Merged via PR #1245 to main.
Merged by deterministic merge pass (pulse-wrapper.sh).

@github-actions
Copy link
Copy Markdown

Performance Test Results

Performance test results for 68cf829 are in 🛎️!

Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown.

URL: /

Run DB Queries Memory Before Template Template WP Total LCP TTFB LCP - TTFB
0 41 37.78 MB 861.00 ms (+22.00 ms / +3% ) 154.00 ms (-9.00 ms / -6% ) 1022.50 ms (-53.50 ms / -5% ) 1950.00 ms 1870.95 ms 80.55 ms
1 56 49.13 MB 943.00 ms (+55.00 ms / +6% ) 139.50 ms 1086.50 ms (+56.50 ms / +5% ) 2088.00 ms (+50.00 ms / +2% ) 2014.50 ms (+47.00 ms / +2% ) 72.50 ms

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

Labels

origin:worker Auto-created by pulse labelless backfill (t2112) review-feedback-scanned Merged PR already scanned for quality feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quality-debt: assets/js/template-previewer.js — PR #1222 review feedback (high)

1 participant