Skip to content

Use IFRAME to display HTML responses for REST API storage request failures in Site Health test - #1849

Merged
westonruter merged 8 commits into
WordPress:trunkfrom
SohamPatel46:add/iframe-for-raw-response
Feb 6, 2025
Merged

Use IFRAME to display HTML responses for REST API storage request failures in Site Health test#1849
westonruter merged 8 commits into
WordPress:trunkfrom
SohamPatel46:add/iframe-for-raw-response

Conversation

@SohamPatel46

@SohamPatel46 SohamPatel46 commented Feb 5, 2025

Copy link
Copy Markdown
Contributor

Summary

Fixes #1828

Relevant technical choices

  • If the raw response for Optimization Detective storage request failure has a text/html type, use an IFRAME combined with sandbox and seamless attributes.
  • Replace the wrapper functionwp_admin_notice with wp_get_admin_notice to whitelist the used IFRAME and posts parameters using wp_kses function.

Screenshots or screencast

Screenshot 2025-02-05 at 2 52 51 PM
@github-actions

github-actions Bot commented Feb 5, 2025

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: SohamPatel46 <sohampate1@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@codecov

codecov Bot commented Feb 5, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.30%. Comparing base (68c6deb) to head (c748645).
Report is 28 commits behind head on trunk.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #1849      +/-   ##
==========================================
+ Coverage   65.97%   66.30%   +0.33%     
==========================================
  Files          88       88              
  Lines        6895     6963      +68     
==========================================
+ Hits         4549     4617      +68     
  Misses       2346     2346              
Flag Coverage Δ
multisite 66.30% <100.00%> (+0.33%) ⬆️
single 37.55% <9.52%> (-0.62%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Optimization Detective Issues for the Optimization Detective plugin labels Feb 5, 2025
Comment thread plugins/optimization-detective/site-health.php Outdated
$result['description'] .= '<details><summary>' . esc_html__( 'Raw response:', 'optimization-detective' ) . '</summary><pre style="white-space: pre-wrap">' . esc_html( $body ) . '</pre></details>';
$result['description'] .= '<details><summary>' . esc_html__( 'Raw response:', 'optimization-detective' ) . '</summary>';

if ( isset( $header ) && 'text/html' === $header ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will account for headers values like text/html; charset=utf-8 and application/xml+xhtml:

Suggested change
if ( isset( $header ) && 'text/html' === $header ) {
if ( is_string( $header ) && str_contains( $header, 'html' ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Aside: $header will always be set and never be null so the use of isset() probably was unnecessary, right?

Comment thread plugins/optimization-detective/site-health.php
Comment thread plugins/optimization-detective/site-health.php
@westonruter

Copy link
Copy Markdown
Member

I found the size of the IFRAME was a bit small:

Screenshot 2025-02-06 10 43 16

So in 6685c7a I increased the size:

Screenshot 2025-02-06 10 43 37

@westonruter westonruter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@westonruter
westonruter force-pushed the add/iframe-for-raw-response branch from 5c161b2 to 0307bb5 Compare February 6, 2025 19:20
@westonruter
westonruter force-pushed the add/iframe-for-raw-response branch from 0307bb5 to c748645 Compare February 6, 2025 19:21
@westonruter westonruter changed the title Use IFRAME to display raw response for Optimization Detective storage request failure Feb 6, 2025
@westonruter
westonruter merged commit e845dd8 into WordPress:trunk Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature

2 participants