Skip to content

pad: clarify that read-only sharing selects a link - #8326

Open
jum-apzn wants to merge 2 commits into
ether:developfrom
jum-apzn:fix/clarify-read-only-share-link
Open

jum-apzn wants to merge 2 commits into
ether:developfrom
jum-apzn:fix/clarify-read-only-share-link

Conversation

@jum-apzn

Copy link
Copy Markdown

Fixes #8181.

What and why

The Share dialog's “Read only” checkbox selects the URL and embed code to copy; it does not lock the pad. Rename the label to “Use read-only link” and add a localized explanation that the editable link remains usable. Associate the explanation with the checkbox using aria-describedby, preserving the existing label and write-only visibility scope.

Add Playwright coverage for the rendered English label and accessible description, link/embed switching, and continued access through both URLs. No permission logic or non-English locale files are changed.

Validation

  • TypeScript: node_modules/.bin/tsc --noEmit (run from src): passed.
  • pnpm run test-utils: 20 passing.
  • English locale parsing, template structure checks, and git diff --check: passed.
  • Playwright discovery found the 2 new tests.
  • The 2 new tests and 5 existing embed_value.spec.ts tests were attempted, but Chromium failed to start with socket() failed: Operation not permitted (1). All 7 stopped before assertions; browser E2E and visual validation remain unverified.
  • pnpm run lint failed during configuration because ESLint 10 requires eslint.config.*, while the repository currently uses .eslintrc.cjs. No lint configuration changes are included.
  • The full frontend/backend suites were not completed.

AI assistance and review

Prepared and reviewed with OpenAI Codex assistance. The final patch was reviewed and approved by a human before submission.

Clarify the Share checkbox label and add a localized, accessible explanation that it changes only the share link and embed code. Include Playwright regressions covering label/description, URL and iframe switching, and editability through both links.
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Clarify that read-only sharing selects a link, not pad permissions

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Clarify that the Share checkbox changes the copied link and embed code, not pad permissions.
• Add an accessible explanation that the editable link still permits changes.
• Test the label, link switching, and access through both URLs.
Diagram

graph TD
  Share["Share dialog"] --> Checkbox["Link checkbox"] --> Choice{"Read-only selected?"} -->|Yes| ReadOnly["Read-only link"] --> Embed["Matching embed"]
  Choice -->|No| Editable["Editable link"] --> Embed
Loading
High-Level Assessment

Keep this as a localized Share-dialog clarification: the existing checkbox already switches URLs and embed code without changing permissions. Changing permission logic would address a different problem, while a label change alone would not explain that the editable link remains usable.

Files changed (3) +59 / -2

Bug fix (2) +4 / -2
en.jsonClarify English read-only sharing text +2/-1

Clarify English read-only sharing text

• Renames the checkbox label to “Use read-only link” and adds an explanation that it changes only the displayed link and embed code. The text also warns that anyone with the editable link can still edit.

src/locales/en.json

pad.htmlAssociate the Share explanation with its checkbox +2/-1

Associate the Share explanation with its checkbox

• Adds a localized explanation inside the existing write-only Share control and connects it to the checkbox with aria-describedby. The existing label association and visibility scope remain intact.

src/templates/pad.html

Tests (1) +55 / -0
share_readonly.spec.tsCover Share wording, link switching, and URL access +55/-0

Cover Share wording, link switching, and URL access

• Adds Playwright tests for the English label and accessible description, link and embed-code switching, and access through read-only and editable URLs. The browser tests were not verified in the reported environment because Chromium could not start.

src/tests/frontend-new/specs/share_readonly.spec.ts

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Restricted viewers are promised editing ✓ Resolved
Description
pad.share.readonly-explanation says anyone with the editable link can edit, but the link does not
override per-user restrictions. When authentication is required, userCanModify() rejects read-only
users and users without modification authorization even if they open that URL.
Code

src/locales/en.json[368]

+  "pad.share.readonly-explanation": "This only changes the link and embed code shown here. Keep the editable link to make changes; anyone with that link can still edit the pad.",
Evidence
The new guidance makes an unconditional editing promise. The authorization check and socket setup
show that an editable URL can still result in read-only access for a restricted user, undermining
the rule's requirement for clear access guidance.

Clear read-only URL and toggle guidance
src/locales/en.json[368-368]
src/node/hooks/express/webaccess.ts[57-65]
src/node/handler/PadMessageHandler.ts[493-498]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Share dialog promises that anyone with the editable link can edit, even though authenticated users may have read-only access.
## Fix Focus Areas
- src/locales/en.json[368-368]
## Recommended Fix
Revise the explanation to say the editable link remains usable for people who have permission to edit, while retaining the distinction between selecting a link and changing pad permissions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/locales/en.json Outdated
Clarify that retaining the editable link does not bypass existing edit permissions. Update the matching regression-test expectation.

This branch has not been deployed

No deployments
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.

Please improve read-only usage

1 participant