Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 68 additions & 39 deletions src/social-share-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,16 @@ class SocialShareButton {

this.button = button;
if (this.options.container) {
const container =
typeof this.options.container === "string"
? document.querySelector(this.options.container)
: this.options.container;
let container = null;
try {
// DOM manipulation: querySelector may throw for invalid selectors
container =
typeof this.options.container === "string"
? document.querySelector(this.options.container)
: this.options.container;
} catch (error) {
console.error("DOM manipulation failed: Invalid container selector", error);
Comment on lines +96 to +97
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 | 🟠 Major

Replace bare console.error calls with a debug-gated helper.

These catch blocks now write directly to consumers' browser consoles. In this repo that both violates the approved library logging pattern and trips the project's no-console rule; reuse or generalize a single private helper gated by options.debug instead.

Based on learnings: bare console statements in library catch blocks are unacceptable in production, the approved pattern is a debug-gated _debugWarn(message, err) helper, and eslint.config.js already enforces no-console: error.

Also applies to: 417-418, 428-429, 435-436, 487-493, 531-532

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/social-share-button.js` around lines 96 - 97, Replace all bare
console.error calls in the catch blocks of src/social-share-button.js with the
library's debug-gated helper `_debugWarn(message, err)` (or create it if
missing) so logging is only emitted when options.debug is truthy; specifically,
change the catch in the DOM container selector error (current catch around the
"DOM manipulation failed: Invalid container selector" message) and the other
listed catch blocks (around lines 417-418, 428-429, 435-436, 487-493, 531-532)
to call `_debugWarn("... descriptive message ...", error)` instead of
console.error, and implement `_debugWarn` as a private function that checks
`options.debug` before calling console.* to satisfy the `no-console` rule and
match the approved pattern.

}

if (container) {
container.appendChild(button);
Expand Down Expand Up @@ -404,15 +410,31 @@ class SocialShareButton {
}

share(platform) {
const shareUrl = this.getShareURL(platform);
let shareUrl = "";
try {
// URL handling: encodeURIComponent may throw URIError on lone surrogates
shareUrl = this.getShareURL(platform);
} catch (error) {
console.error("URL handling failed: Data encoding error", error);
}
Comment on lines +413 to +419
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 | 🟠 Major

Do not report social_share_success after a caught share error.

If URL generation or navigation/open throws, the new catches swallow the failure and this method still falls through to either social_share_success or the generic "No share URL configured" branch. That makes the analytics payload incorrect relative to src/social-share-analytics.js:19-75, and onShare can run for a failed share. Emit social_share_error with the caught message and return from the catch path before the success/onShare branch.

Also applies to: 425-449

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/social-share-button.js` around lines 413 - 419, When URL generation or
navigation fails in the share flow, do not let the function continue to the
success branch; inside the catch blocks around this.getShareURL(platform) and
the later navigation/open code, emit the analytics event social_share_error
(including the caught error message) and return immediately to avoid running
onShare or emitting social_share_success or the "No share URL configured"
branch; update the catch handlers that currently log errors to call the
analytics emitter (matching the payload shape used in
src/social-share-analytics.js) with event name social_share_error and then
return from the enclosing method instead of falling through.


if (shareUrl) {
this._emit("social_share_click", "share", { platform });

if (platform === "email") {
window.location.href = shareUrl;
try {
// Navigation may be blocked in some restricted contexts
window.location.href = shareUrl;
} catch (error) {
console.error("Sharing failed: Navigation strictly blocked", error);
}
} else {
window.open(shareUrl, "_blank", "noopener,noreferrer,width=600,height=600");
try {
// window.open may throw if blocked by browser settings or in restricted frames
window.open(shareUrl, "_blank", "noopener,noreferrer,width=600,height=600");
} catch (error) {
console.error("Sharing failed: Popup window blocked", error);
}
}

this._emit("social_share_success", "share", { platform });
Expand All @@ -434,37 +456,43 @@ class SocialShareButton {

// Check if clipboard API is available
if (navigator.clipboard && navigator.clipboard.writeText) {
navigator.clipboard
.writeText(this.options.url)
.then(() => {
// Guard against async callback after destroy
if (this.isDestroyed) return;

copyBtn.textContent = "Copied!";
copyBtn.classList.add("copied");
this._emit("social_share_copy", "copy");

if (this.options.onCopy) {
this.options.onCopy(this.options.url);
}

// Clear any existing feedback timeout
if (this.feedbackTimeout) {
clearTimeout(this.feedbackTimeout);
}

// Track feedback timeout to prevent callback after destroy
this.feedbackTimeout = setTimeout(() => {
if (this.isDestroyed || !copyBtn) return; // Safety check
copyBtn.textContent = "Copy";
copyBtn.classList.remove("copied");
this.feedbackTimeout = null;
}, 2000);
})
.catch(() => {
// Fallback to manual selection
this.fallbackCopy(input, copyBtn);
});
try {
navigator.clipboard
.writeText(this.options.url)
.then(() => {
// Guard against async callback after destroy
if (this.isDestroyed) return;

copyBtn.textContent = "Copied!";
copyBtn.classList.add("copied");
this._emit("social_share_copy", "copy");

if (this.options.onCopy) {
this.options.onCopy(this.options.url);
}

// Clear any existing feedback timeout
if (this.feedbackTimeout) {
clearTimeout(this.feedbackTimeout);
}

// Track feedback timeout to prevent callback after destroy
this.feedbackTimeout = setTimeout(() => {
if (this.isDestroyed || !copyBtn) return; // Safety check
copyBtn.textContent = "Copy";
copyBtn.classList.remove("copied");
this.feedbackTimeout = null;
}, 2000);
})
.catch((error) => {
console.error("Clipboard copy failed async:", error);
// Fallback to manual selection
this.fallbackCopy(input, copyBtn);
});
} catch (error) {
console.error("Clipboard wrapper failed synchronously:", error);
this.fallbackCopy(input, copyBtn);
}
} else {
// Fallback for browsers without clipboard API
this.fallbackCopy(input, copyBtn);
Expand Down Expand Up @@ -500,7 +528,8 @@ class SocialShareButton {
copyBtn.classList.remove("copied");
this.feedbackTimeout = null;
}, 2000);
} catch (_err) {
} catch (error) {
console.error("Clipboard copy failed (fallback document.execCommand):", error);
copyBtn.textContent = "Failed";

// Clear any existing feedback timeout
Expand Down
Loading