Skip to content

fix: replace iframe with link in build_inspect_data - #485

Open
mvanhorn wants to merge 1 commit into
compiler-research:mainfrom
mvanhorn:fix/484-inspect-iframe-link
Open

fix: replace iframe with link in build_inspect_data#485
mvanhorn wants to merge 1 commit into
compiler-research:mainfrom
mvanhorn:fix/484-inspect-iframe-link

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Summary

  • Replaces the <iframe> in build_inspect_data (src/xinspect.cpp) with a plain <a> anchor that opens the documentation URL in a new tab
  • Removes the now-unused #pager-container / .xcpp-iframe-pager CSS block
  • Adds a unit test in test/test_interpreter.cpp that asserts the returned text/html contains an <a href> and no <iframe>

Why this matters

Closes #484. cppreference.com migrated to MediaWiki, which sets X-Frame-Options: DENY by default as a deliberate security policy. This blocks the <iframe> pager in every browser (Chrome, Safari, JupyterLite), leaving the Shift+Tab inspect panel blank for all users. The maintainer confirmed that changing the cppreference server config is not feasible, so the fix must be on the xeus-cpp side. A clickable link is the simplest correct replacement - it requires no special permissions and works in all Jupyter environments.

Fixes #484

cppreference.com sets X-Frame-Options: DENY (MediaWiki default), which
causes every browser to block the <iframe> used in build_inspect_data.
Replace it with a plain <a> anchor that opens the documentation URL in
a new tab, keeping the existing text/plain field intact.

Adds a unit test asserting the output contains an <a href> and no
<iframe>.

Fixes compiler-research#484
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.01%. Comparing base (82f9064) to head (e23f5ed).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #485      +/-   ##
==========================================
+ Coverage   73.99%   74.01%   +0.02%     
==========================================
  Files          23       23              
  Lines        1196     1197       +1     
  Branches      111      111              
==========================================
+ Hits          885      886       +1     
  Misses        311      311              
Files with missing lines Coverage Δ
src/xinspect.cpp 91.96% <100.00%> (+0.07%) ⬆️
Files with missing lines Coverage Δ
src/xinspect.cpp 91.96% <100.00%> (+0.07%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@anutosh491 anutosh491 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious as to how the deployment looks like (maybe share a screencast or an image ?)

We also have a solution here : #484 (comment)

@mvanhorn

Copy link
Copy Markdown
Contributor Author

xeus-cpp inspect pager: before / after

I could not capture this inside a live xeus-cpp deployment, both linked JupyterLite demos currently load Lab but expose no C++ kernel, so I reproduced the pager payloads directly in Chrome against en.cppreference.com. Same HTML, same origin behaviour.

The result changed my mind about this PR, so let me lead with that.

The blocker is ?action=purge, not cppreference:

$ curl -sI "https://en.cppreference.com/w/cpp/container/vector?action=purge" | grep -i x-frame
x-frame-options: DENY

$ curl -sI "https://en.cppreference.com/cpp/container/vector" | grep -i x-frame
(nothing)

Article views send no X-Frame-Options at all today. Only the ?action=purge URL does, and that suffix is appended by build_inspect_data itself. Pane 1 above is the current code, showing the blocked-document icon; pane 2 is the identical iframe with ?action=purge removed, rendering the full std::vector page.

So there is a third option nobody in #484 has raised, and I think it beats both on the table: delete ?action=purge and keep the iframe. One line, keeps documentation inline where @mcbarton wants it, and needs no mirror, no hosting and no CC-BY-SA determination.

Two caveats. ?action=purge was presumably there to defeat MediaWiki caching, so dropping it means a stale page could be served; for read-only documentation that seems a fair trade, but it is your call. And these are the headers cppreference serves today, so if they add X-Frame-Options to article views later, the iframe breaks again and the link approach in this PR becomes right after all.

Happy to reshape #485 into the one-line removal if you agree, keeping the test but asserting the emitted src no longer carries the purge suffix.

Separately, unrelated to this PR: both https://compiler-research.github.io/xeus-cpp/lab/index.html and https://compiler-research.org/xeus-cpp-wasm/lab/index.html load Lab but offer no C++ kernel in the launcher or kernel picker, so notebooks open with "No Kernel". Might be worth a look.

@mcbarton

Copy link
Copy Markdown
Collaborator

Separately, unrelated to this PR: both https://compiler-research.github.io/xeus-cpp/lab/index.html and https://compiler-research.org/xeus-cpp-wasm/lab/index.html load Lab but offer no C++ kernel in the launcher or kernel picker, so notebooks open with "No Kernel". Might be worth a look.

@mvanhorn I tried both deployments and the kernels are there for me.

In the past when the kernels have not appeared for me it was because I changed the config on my browser and that somehow effected it. No idea why. I just reset my browser config to the defaults and they appeared again. I say this not knowing if it will help you.

@mcbarton

mcbarton commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

I am for a technique which avoids opening the documentation in a new tab or window. I have been learning about accessibility in web pages recently, and Opening new windows and tabs from a link only when necessary is something which is advised (see https://www.w3.org/WAI/WCAG22/Techniques/general/G200)

@anutosh491

anutosh491 commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Hey @mvanhorn,

We would love if the behaviour is as close as what we had originally i.e the docs render below the cell that calls for it !

If you're approach can get us to that, let's go for it.

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.

[Bug] inspect request failure

4 participants