Skip to content

Fix copy button on Mobile Safari, simplify implementation#171

Merged
simonw merged 1 commit intomainfrom
xenodochial-meninsky
Dec 20, 2025
Merged

Fix copy button on Mobile Safari, simplify implementation#171
simonw merged 1 commit intomainfrom
xenodochial-meninsky

Conversation

@simonw
Copy link
Owner

@simonw simonw commented Dec 20, 2025 •
edited
Loading

Remove touchstart handler that was intercepting button taps and calling synthetic click(), which broke the user gesture chain required by Mobile Safari for clipboard access.

Simplify copy button to match text-indentation.html pattern.

https://tools.simonwillison.net/claude-code-timeline?url=https%3A%2F%2Fgist.githubusercontent.com%2Fsimonw%2F86d8fcbad4f70661616db0d783e1c43e%2Fraw%2Fd2edbbeb3484169aa769e9eb984e99dbc9760c15%2Findex.jsonl#tz=local&q=&type=all&ct=all&role=all&hide=0&truncate=1&sel=81

🤖 Generated with Claude Code

Full prompt sequence:

Update render-markdown to use the same copy button implementation as text-indentation

I still see a weird alert message about pressing ctrl+c, ditch that

Why would I ever see a "Failed to copy" message?

It fails for me on the render-markdown page but works on the text-indentation one - in Mobile Safari on iOS

Fore refreshed and it is still a bug. Could it be related to how the HTML Output area is populated?

how come text-indentation does not need that but render-markdown does?

I'm sure I have built other tools where this worked even though the data being copied was generated by code - take a look at other tools in this repo and review how they work

I still get failed to copy. this is so weird

I don't want a workaround, I want to understand exactly why this happens for that page but not the other

huh I wonder where that touchstart thing came from

yes

Yes that fixed it

@simonw @claude
Fix copy button on Mobile Safari, simplify implementation
30277f6 
Remove touchstart handler that was intercepting button taps and calling
synthetic click(), which broke the user gesture chain required by
Mobile Safari for clipboard access.

Simplify copy button to match text-indentation.html pattern.

https://tools.simonwillison.net/claude-code-timeline?url=https%3A%2F%2Fgist.githubusercontent.com%2Fsimonw%2F86d8fcbad4f70661616db0d783e1c43e%2Fraw%2Fd2edbbeb3484169aa769e9eb984e99dbc9760c15%2Findex.jsonl#tz=local&q=&type=all&ct=all&role=all&hide=0&truncate=1&sel=81

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Dec 20, 2025 •
edited
Loading

Claude finished @simonw's task —— View job

Code Review Analysis

Reviewing PR #171: Fix copy button on Mobile Safari, simplify implementation

Todo List

  • Check for repository guidelines (CLAUDE.md)
  • Read and analyze render-markdown.html changes
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Review test coverage implications
  • Provide comprehensive feedback

✅ Code Review Summary

This PR successfully addresses a critical Mobile Safari clipboard access issue by removing problematic touchstart handlers and simplifying the copy functionality. Overall, this is a well-executed fix that improves both functionality and code maintainability.

📋 Detailed Analysis

Code Quality & Best Practices ⭐ Excellent

  • Simplification: Reduced from 115 lines to 21 lines of JavaScript (+21/-94) - excellent code reduction
  • Consistency: Now matches the proven pattern from text-indentation.html:clipboard
  • Maintainability: Removed complex fallback logic that was causing issues
  • Standards compliance: Uses modern navigator.clipboard.writeText() API correctly

Bug Fixes ✅ Addresses Core Issue

  • Mobile Safari fix: Correctly removes touchstart handler that broke the user gesture chain for clipboard access
  • User gesture preservation: Direct click events now properly maintain the gesture chain required by Mobile Safari
  • Fallback removal: Eliminates problematic document.execCommand('copy') fallback that wasn't working reliably

Performance Considerations ✅ Improved

  • Reduced complexity: Eliminates unnecessary DOM manipulation and multiple code paths
  • Faster execution: Simpler event flow without synthetic click() calls
  • Memory efficiency: Removes unused CSS classes and DOM elements

Security Assessment ✅ No Concerns

  • API usage: Uses secure navigator.clipboard API appropriately
  • No XSS vectors: Clean implementation with no dynamic HTML generation
  • Error handling: Proper try-catch blocks prevent exceptions

Minor Observations

  1. Textarea readonly removal: The readonly attribute was removed from the output textarea (render-markdown.html:354). This allows user editing of the output - verify this is intentional behavior.

  2. Error messaging: The fallback error message "Failed to copy" is clear and user-friendly.

  3. Browser compatibility: Modern approach assumes navigator.clipboard support (IE11+ not supported, which is reasonable for 2025).

Test Coverage Recommendations

While no automated tests exist for this functionality, consider manual testing:

  • Mobile Safari clipboard access
  • Desktop browser compatibility
  • Error handling when clipboard access is denied
  • Button state transitions (visual feedback)

🎯 Verdict

Approved - This is a focused, well-implemented fix that addresses the specific Mobile Safari issue while significantly simplifying the codebase. The changes follow established patterns and improve both functionality and maintainability.

@simonw
Copy link
Owner Author

simonw commented Dec 20, 2025

Clicking "Copy HTML" on the https://tools.simonwillison.net/render-markdown on iOS was failing with an error, turns out the culprit was this:

tools/render-markdown.html

Lines 594 to 600 in de78b50

// Add double-tap zoom prevention
document.addEventListener('touchstart', function(event) {
if (event.target.tagName === 'BUTTON' || event.target.tagName === 'INPUT') {
event.preventDefault();
event.target.click();
}
}, { passive: false });

@simonw simonw merged commit 8bd1066 into main Dec 20, 2025
4 checks passed
@simonw simonw deleted the xenodochial-meninsky branch December 20, 2025 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@simonw