Fix GoDAM 2.1 QA: non-PDF documents, audio overflow, stray media icon, Image rename - #2052
Closed
subodhr258 wants to merge 1 commit into
Closed
Fix GoDAM 2.1 QA: non-PDF documents, audio overflow, stray media icon, Image rename#2052subodhr258 wants to merge 1 commit into
subodhr258 wants to merge 1 commit into
Conversation
Document block (QA 3, 5, 6) render.php emitted `<object type="application/pdf">` for whatever file was selected. Given a non-PDF the browser gets a type it cannot display, so it either paints an empty box, suppressing the `<object>` fallback and leaving nothing at all on screen, or hands the file to the download manager, which starts a download on every page load. Reproduced both: a .docx rendered a 0px-wide object with no fallback, and merely loading the page wrote the file into ~/Downloads without any click. PDF is the only format the block supports, so add godam_is_supported_document() and gate output on it. The shared render.php backs the block, the [godam_document] shortcode and the Elementor widget, so one guard covers all three front ends; each editor now shows an "unsupported format" notice instead of a silently empty element. Also close a hole in the block's selection guard, where a missing MIME type passed straight through. Audio player (QA 2) .godam-audio-player is a nowrap flex row, and the scrubber had flex:1 1 auto without min-width:0. input[type=range] carries a ~129px intrinsic min-content width, so the track refused to shrink. In a narrow card, such as a 3-column layout, play + track + time then needed 241px in a 198px row and the non-shrinking time label was pushed outside the card and the column. Matches how __body and __play in the same file already handle it. Elementor media control (QA 7) Elementor centres .eicon-video-camera over the media area as its empty-state affordance and never hides it, because its own control only ever sets `src`. Ours also sets a poster, which left that icon stranded on top of the thumbnail. Hide it while a poster is showing; keep it for the empty state. Rename "GoDAM Image" to "Image" in core, Elementor and WPBakery, matching the Video, Audio, Document and Video Gallery labels already in the GoDAM category. Tests godam_is_supported_document() is the single gate protecting the embed, so cover its edge cases: MIME beats a misleading .pdf URL, query strings and fragments, a deleted attachment falling through to the URL, and "pdf" appearing outside the extension.
subodhr258
force-pushed
the
fix/godam-2.1-qa-document-audio-icon
branch
from
July 29, 2026 11:50
c11dcd3 to
65e52ed
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes GoDAM 2.1 QA regressions around Document (non-PDF) rendering, Elementor preview UI quirks, and Audio layout overflow, while also renaming “GoDAM Image” to “Image” across editors/builders.
Changes:
- Add
godam_is_supported_document()gating to prevent non-PDF documents from rendering/triggering downloads, plus editor-side “unsupported format” notices. - Fix audio scrubber overflow in narrow (3-column) layouts via flexbox min-width adjustment.
- Clean up Elementor media control preview (hide stray camera icon when poster is present) and apply “Image” rename across Gutenberg/Elementor/WPBakery.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/php/DocumentSupportTest.php | Adds unit tests covering MIME-vs-URL PDF support decisions and edge cases. |
| tests/bootstrap.php | Extends test stubs/constants and loads helper functions under test. |
| inc/helpers/custom-functions.php | Introduces godam_is_supported_document() helper to gate PDF-only embedding. |
| inc/classes/wpbakery-elements/class-wpb-godam-image.php | Renames WPBakery element label to “Image”. |
| inc/classes/shortcodes/class-godam-document.php | Shows unsupported-format notice in WPBakery front-end editor; otherwise renders nothing for non-PDF. |
| inc/classes/elementor-widgets/class-godam-image.php | Renames Elementor widget title to “Image”. |
| inc/classes/elementor-widgets/class-godam-document.php | Adds Elementor editor/preview unsupported-format rendering for non-PDF documents. |
| assets/src/js/elementor/controls/godam-media.js | Hides Elementor’s camera icon overlay when a poster thumbnail is present. |
| assets/src/css/godam-elementor-preview.scss | Styles Elementor unsupported state alongside existing empty-state placeholders. |
| assets/src/css/_godam-audio-card.scss | Fixes 3-column audio overlap by allowing the scrubber to shrink (min-width: 0). |
| assets/src/blocks/godam-pdf/style.scss | Adds WPBakery front-end editor unsupported-format notice styling. |
| assets/src/blocks/godam-pdf/render.php | Prevents non-PDF output at render-time by gating with the new helper. |
| assets/src/blocks/godam-pdf/editor.scss | Adds Gutenberg editor-only unsupported-format notice styling. |
| assets/src/blocks/godam-pdf/edit.js | Strengthens PDF validation, supports in-flight uploads, and adds editor-canvas unsupported notice. |
| assets/src/blocks/godam-image/block.json | Renames Gutenberg block title to “Image”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+409
to
+414
| // ── Editor canvas: unsupported format ───────────────────────────────────── | ||
| const UnsupportedCanvas = ( | ||
| <div className="godam-pdf-unsupported" data-test-id="godam-pdf-content-unsupported"> | ||
| <span className="godam-pdf-unsupported__icon dashicons dashicons-media-document" /> | ||
| <h3 className="godam-pdf-unsupported__title"> | ||
| { __( 'Unsupported file format', 'godam' ) } |
Comment on lines
+298
to
+308
| // Content saved before the format was validated (or added by URL) can still hold | ||
| // a non-PDF. The front end renders nothing for those, so flag it here rather than | ||
| // showing an <embed> that just paints blank. | ||
| // Only flag on positive evidence: a loaded attachment whose MIME is not PDF, or, | ||
| // for a URL-only document, a URL that is not a .pdf. Staying quiet while the | ||
| // attachment record is still loading avoids flashing the notice on a good PDF. | ||
| const isUnsupported = hasDocument && ! temporaryURL && ( | ||
| id | ||
| ? ( !! mimeType && 'application/pdf' !== mimeType ) | ||
| : ! /\.pdf(?:[?#]|$)/i.test( src || '' ) | ||
| ); |
Comment on lines
+64
to
+72
| // PDF is the only supported format; render.php emits nothing for anything | ||
| // else. In WPBakery's front-end editor that would leave a silently empty | ||
| // element, so show the author why. Callers that own their own editor | ||
| // messaging (the Elementor widget) check before reaching this point. | ||
| if ( ! godam_is_supported_document( $atts['id'], $atts['src'] ) ) { | ||
| if ( function_exists( 'vc_is_inline' ) && vc_is_inline() ) { | ||
| // The notice is styled by the block's stylesheet, which is normally | ||
| // enqueued further down, so enqueue it here too since we return early. | ||
| wp_enqueue_style( 'godam-pdf-style' ); |
Comment on lines
+192
to
+198
| // PDF is the only supported format. The shared render.php emits nothing for | ||
| // anything else, which would leave a silently empty widget, so flag it in | ||
| // the editor instead, where the author can act on it. | ||
| if ( ! godam_is_supported_document( isset( $document_file['id'] ) ? $document_file['id'] : 0, $document_file['url'] ) ) { | ||
| $this->render_unsupported_state(); | ||
| return; | ||
| } |
Comment on lines
+48
to
+53
| public function test_attachment_mime_type_is_authoritative() { | ||
| $this->stub_mime( 'application/pdf' ); | ||
| $this->assertTrue( | ||
| godam_is_supported_document( 201, 'https://example.com/report.pdf' ), | ||
| 'A PDF attachment should be supported.' | ||
| ); |
Collaborator
Author
|
Superseded by #2053. The branch was renamed to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes five items from the GoDAM 2.1 QA tab plus one naming request from Slack. All were reproduced locally on Elementor 4.2.1 + WPBakery (js_composer) 9.0-beta.3 + WooCommerce before any code was written.
QA 3, 5, 6: non-PDF documents
render.phpemitted<object type="application/pdf">pointing at whichever file was selected. Given a non-PDF the browser receives a type it cannot display, and the outcome splits by browser setting:<object>fallback is suppressed, so nothing at all is paintedEvidence for QA 5: loading the page, with no click at all, wrote a file into
~/Downloadsthat was byte-identical to the test.docx(945 bytes, same zip entries, same body text).PDF is the only format this block supports, so
godam_is_supported_document()now gates output. The attachment's stored MIME type is authoritative, with the URL extension as a fallback for GoDAM tab media and URL-only documents.The Gutenberg block, the
[godam_document]shortcode (WPBakery) and the Elementor widget all render through the samerender.php, so one guard covers all three front ends. Because rendering nothing would otherwise leave an author staring at an empty element, each editor now shows an "unsupported format" notice instead.Also closed a hole in the block's own selection guard: it read
if ( mime && mime !== 'application/pdf' ), so a missing MIME type passed straight through. It now tests for a positive PDF match and falls back to the extension.QA 2: audio overlap in 3 columns
.godam-audio-playeris anowrapflex row with a 14px gap.__scrubberhadflex: 1 1 autobut nomin-width: 0, andinput[type=range]carries a ~129px intrinsic min-content width, so the track refused to shrink:The 43px excess pushed the non-shrinking
__timelabel outside the card and 8.1px past the Elementor column. It only shows in 3-column layouts because wider columns have slack. Addingmin-width: 0shrinks the track to 86px and the row fits; the same file already does exactly this on__bodyand__play.QA 7: stray icon over the thumbnail
Elementor absolutely centres
.eicon-video-cameraover the media area as its empty-state affordance and never hides it, because its own control only ever setssrcon the preview<video>. Our control additionally setsposterfrom the GoDAM thumbnail, which left that icon stranded on top of a real image (measured 21x21, visible, dead centre over the 258x145 poster, withe-media-emptyabsent). The icon is now hidden while a poster is showing and kept for the empty state.Worth noting for reviewers: the markup in
class-godam-media.phpis byte-identical to Elementor's native control and we add no CSS of our own, so this is only a bug for us because we are the ones adding a poster.Rename
"GoDAM Image" to "Image" in core, Elementor and WPBakery, matching Video / Audio / Document / Video Gallery / Shoppable Video, which already drop the prefix inside the GoDAM category.
One behaviour change to be aware of: typing "godam" into WPBakery's Add Element search no longer matches this element, because no GoDAM element now carries "GoDAM" in its name. The GoDAM category tab remains the discovery path and lists all six correctly. Say the word if you would rather keep it searchable.
Testing
tests/php/DocumentSupportTest.phpcovers the new helper, which is the single gate protecting the embed: MIME beating a misleading.pdfURL, other media types, the URL fallback, query strings and fragments, a deleted attachment falling through to its URL, malformed input, and "pdf" appearing outside the extension.Confirmed the tests can fail: mutating the helper to always return
truefails all 7.tests/bootstrap.phpgains awp_parse_urlstub, WP time constants, and a require ofcustom-functions.php.Manual verification steps
Needs both Elementor and WPBakery active, plus a non-PDF (for example a
.docx) and a PDF in the media library..docxin a Document block, a[godam_document]shortcode and an Elementor Document widget. Each should render nothing at all, and no download should start on load or refresh. Check with the browser's PDF viewer both enabled and disabled (chrome://settings/content/pdfDocuments), since the old bug presented differently in each..docxblock shows "Unsupported file format" with the file name instead of a blank embed. Selecting a non-PDF from the library shows the snackbar warning and does not attach./godamin the Gutenberg inserter, the Elementor widget panel, and WPBakery's Add Element > GoDAM tab all read "Image".Automation impact
New hooks for E2E:
godam-pdf-content-unsupported(Gutenberg canvas notice)godam-document-elementor-unsupported(Elementor editor notice)godam-document-unsupported(WPBakery front-end editor notice)No existing
data-test-idwas renamed or removed. Existing document selectors are unchanged for PDFs; suites that assert on a rendered non-PDF document will now correctly find no output.