Skip to content

fileuploader fixed - #121

Open
veronika-tseleva-cleantalk wants to merge 3 commits into
devfrom
fix_55346
Open

fileuploader fixed#121
veronika-tseleva-cleantalk wants to merge 3 commits into
devfrom
fix_55346

Conversation

@veronika-tseleva-cleantalk

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the automatic screenshot flow in FileUploader to improve reliability when capturing pages that can taint canvases (cross-origin CSS/images/iframes), preferring dom-to-image-more when the page appears “safe” and falling back to html2canvas with sanitization.

Changes:

  • Added a “page safety” pre-check to decide whether to attempt dom-to-image-more.
  • Adjusted html2canvas options and added an onclone hook to remove/replace elements that can taint screenshots.
  • Updated the built bundle to reflect the source changes.

Reviewed changes

Copilot reviewed 1 out of 4 changed files in this pull request and generated 4 comments.

File Description
js/src/fileuploader.js Adds safety detection and refactors screenshot capture to prefer dom-to-image-more and sanitize html2canvas rendering.
dist/doboard-widget-bundle.js Regenerated bundle including the updated screenshot logic.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread js/src/fileuploader.js
Comment thread js/src/fileuploader.js
Comment thread js/src/fileuploader.js
Comment thread js/src/fileuploader.js Outdated
Copilot AI review requested due to automatic review settings August 20, 2026 12:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

js/src/fileuploader.js:425

  • sheet.cssRules is only being read to detect cross-origin stylesheets, but the assigned rules variable is unused. This can trigger no-unused-vars linting and makes the intent less clear; just access the property without assigning it (or use void).
            try {
                const rules = sheet.cssRules;
            } catch (e) {

js/src/fileuploader.js:444

  • isPageSafeForDomToImage() returns null on URL parsing errors, but callers treat the result as a boolean (if (isSafe)). Returning a non-boolean makes behavior harder to reason about; returning false keeps the contract consistent and still routes to the html2canvas fallback.
            } catch (e){
                return null;
            }

js/src/fileuploader.js:457

  • The guard condition also covers an uninitialized/invalid this.files (!this.files || !Array.isArray(this.files)), but the log message only mentions the file-count limit. This can mislead debugging when the uploader isn't initialized yet.
        if (!this.files || !Array.isArray(this.files) || this.files.length >= this.maxFiles) {
            console.log('SpotFix: File count limit reached.');
            return;

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.

3 participants