Skip to content

Fix bookmarklet on mobile browsers#100

Merged
kraftbj merged 18 commits intotrunkfrom
investigate-issue-50
Apr 3, 2026
Merged

Fix bookmarklet on mobile browsers#100
kraftbj merged 18 commits intotrunkfrom
investigate-issue-50

Conversation

@kraftbj
Copy link
Copy Markdown
Collaborator

@kraftbj kraftbj commented Mar 16, 2026

Summary

  • When window.open() fails (common on mobile browsers like Firefox Mobile/Fenix), falls back to storing scraped data in window.name and navigating the current tab
  • window.name persists across cross-origin navigations within the same tab, has no size limit, and keeps data out of URLs (avoiding leaks to browser history, server logs, and session restore)
  • PHP detects &wn=1 and sets a windowNameMode flag — no server-side data parsing needed
  • React app reads window.name on load, clears it immediately, and processes data through a shared processScrapedData callback (same pipeline as the postMessage path)
  • wn param stripped from the URL bar via replaceState
  • The processScrapedData callback is extracted from the postMessage handler for reuse by both transport paths

Why window.name instead of URL parameters?

The previous approach encoded scraped data as JSON in a _data URL parameter. This had two problems:

  1. Confidentiality: scraped content (selected text, titles, metadata) leaked into browser history, server access logs, and session restore
  2. URL length: browsers truncate around 8KB, requiring progressive payload reduction and silent failure for large content

window.name eliminates both issues — it lives only in tab memory (no logs, no history) and supports multi-MB payloads.

Why not POST form / sessionStorage / REST stash?

  • POST form: SameSite=Lax cookies aren't sent on cross-site form POST — user gets redirected to login
  • sessionStorage: per-origin — bookmarklet runs on the source site, data isn't accessible on the admin site
  • REST stash: same SameSite cookie issue for the POST, plus server-side storage complexity

Compatibility note

PR #90 (HTML selection capture) touches the same files. When both merge, the second will need standard merge conflict resolution and a bookmarklet.min.js rebuild. The sel_html field is automatically included in the full scrapedData payload sent via window.name — no additional field mapping is needed.

Test plan

  • Existing tests pass (PHP + JS)
  • New tests: window.name mode flag detection, postMessage/windowName mode independence, bookmarklet fallback path verification
  • Manual: force popup = null in bookmarklet.js, verify current-tab navigation loads editor with scraped content from window.name
  • Manual: verify normal popup path still works unchanged on desktop
  • Manual: verify wn param is stripped from the URL bar after load
  • Manual: verify browser history does NOT contain scraped content
  • Manual: test with large selections (multiple KB) — no truncation

Fixes #50

When window.open() fails (common on mobile browsers like Firefox
Mobile), fall back to navigating the current tab with scraped data
encoded as JSON in a _data URL parameter. The PHP backend parses
and sanitizes this inline data through the same limit_* methods
used for POST data, and the React app skips the postMessage
listener when inline data mode is active.

Arrays are trimmed (5 images, 3 embeds) in the fallback path to
keep URL length manageable.

See #50
@kraftbj kraftbj added this to the 2.1.0 milestone Mar 16, 2026
@kraftbj kraftbj requested a review from Copilot March 16, 2026 23:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 improves Press This bookmarklet compatibility on mobile browsers where window.open() commonly fails by introducing a current-tab navigation fallback that carries scraped data inline.

Changes:

  • Add a popup-blocked fallback in the bookmarklet that navigates the current tab and passes scraped data via a _data JSON URL parameter (with some array trimming).
  • Extend the PHP bootstrap to parse/merge _data using existing limit_* sanitizers and expose an inlineDataMode flag to the React app.
  • Update the React app to skip the postMessage listener when inline data mode is active, and add tests for the new behavior.

Reviewed changes

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

Show a summary per file
File Description
assets/bookmarklet.js Adds popup-blocked fallback to current-tab navigation with _data JSON payload and trims large arrays.
assets/bookmarklet.min.js Regenerates minified bookmarklet to include the fallback behavior.
class-wp-press-this-plugin.php Parses _data, merges sanitized inline values into $data, tracks inlineDataMode, and adjusts proxy/content generation behavior.
src/App.js Skips postMessage listening when inlineDataMode is enabled.
tests/bookmarklet/bookmarklet.test.js Adds assertions that fallback navigation and trimming logic exist in the bookmarklet source.
tests/php/test-integration.php Adds integration tests covering _data parsing, precedence rules, and invalid JSON handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread class-wp-press-this-plugin.php Outdated
Comment thread class-wp-press-this-plugin.php Outdated
Comment thread assets/bookmarklet.js Outdated
Comment thread src/App.js Outdated
kraftbj added 4 commits March 16, 2026 18:55
- Respect enable_press_this_media_discovery filter for inline data path
- Build minimal fallback payload (only used meta keys, canonical/shortlink)
  with 7500-char URL cap to avoid browser truncation
- Override pm=0 in fallback URL so app doesn't wait for postMessage
- Strip _data param from browser history via replaceState

Fixes #50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 adds a mobile-friendly fallback for the Press This bookmarklet when window.open() is blocked (common on mobile browsers), by encoding a minimal scraped payload into a _data URL param that the PHP backend can merge and the React app can handle without waiting for postMessage.

Changes:

  • Add bookmarklet fallback path that navigates the current tab with _data JSON and enforces a URL-length budget.
  • Add PHP support to parse/merge/sanitize _data via existing limit_*/processing paths and expose inlineDataMode to the client.
  • Update the React app to skip postMessage handling in inline mode and to strip _data from browser history; add tests for the new paths.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
assets/bookmarklet.js Adds popup-blocked fallback to current-tab navigation with _data JSON payload.
assets/bookmarklet.min.js Rebuilt/minified bookmarklet including the fallback logic.
class-wp-press-this-plugin.php Parses _data, sanitizes/merges into the normal data structure, and exposes inlineDataMode.
src/App.js Strips _data from history and skips postMessage listener in inline mode.
tests/bookmarklet/bookmarklet.test.js Adds assertions for fallback behavior and URL-length logic presence.
tests/php/test-integration.php Adds integration tests for _data parsing, precedence, and invalid JSON handling.
package-lock.json Updates the typescript entry metadata (devOptionaldev).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/App.js Outdated
Comment thread assets/bookmarklet.js Outdated
kraftbj added 4 commits March 31, 2026 15:36
When window.open() is blocked on mobile browsers, the bookmarklet now
stores scraped data in window.name instead of encoding it as JSON in
a _data URL parameter. This solves two problems:

- Confidentiality: scraped content no longer leaks into browser
  history, server access logs, or session restore
- URL length: window.name has no practical size limit, eliminating
  the need for progressive payload reduction and silent truncation

The React app reads window.name on load (signaled by &wn=1), clears
it immediately, and processes data through the same pipeline as the
postMessage path. The shared processScrapedData callback is extracted
from the postMessage handler for reuse.

Removes ~90 lines of PHP inline data parsing code that is no longer
needed since data processing happens entirely client-side.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 5 out of 7 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/php/test-integration.php
Comment thread class-wp-press-this-plugin.php Outdated
Comment thread assets/bookmarklet.js Outdated
Comment thread assets/bookmarklet.js Outdated
kraftbj added 3 commits April 2, 2026 08:57
- Set window_name_mode via single assignment (defensive against
  multiple merge_or_fetch_data calls)
- Restore $_SERVER['REQUEST_METHOD'] in test helper to avoid state leaks
- Soften window.name size limit claim in bookmarklet comment
- Fix stale docblock on window_name_mode property
An inline script in the PHP template now reads window.name into
window.__ptWindowName and clears window.name immediately, before
any admin hook scripts execute. The React app reads from the stashed
copy instead of window.name directly.

This narrows the exposure window for attacker-prefilled window.name
payloads and prevents same-origin admin scripts from accessing
scraped data.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 5 out of 7 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/php/test-integration.php
Comment thread tests/php/test-integration.php Outdated
Comment thread assets/bookmarklet.js Outdated
Comment thread class-wp-press-this-plugin.php Outdated
@kraftbj kraftbj marked this pull request as ready for review April 2, 2026 16:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 5 out of 7 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread assets/bookmarklet.js Outdated
Comment thread src/App.js
Comment thread src/App.js
Comment thread tests/php/test-integration.php
Only navigate with &wn=1 when JSON serialization succeeds,
preventing the app from parsing stale or attacker-controlled
window.name data. Add event.origin check on the postMessage
listener and a 1 MB size cap before parsing window.name.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 5 out of 7 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/App.js
Comment thread src/App.js Outdated
kraftbj added 2 commits April 2, 2026 12:18
The bookmarklet controls what goes into window.name, so a size
cap on the receiving end is redundant.
Read back window.name after writing to detect silent browser
truncation. If the payload didn't survive intact, navigate
without &wn=1 so the user gets the normal URL flow instead
of a silently empty editor.
@kraftbj kraftbj added this pull request to the merge queue Apr 3, 2026
Merged via the queue into trunk with commit a55f050 Apr 3, 2026
6 checks passed
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.

Fix bookmarklet on mobile browsers, eg Firefox

2 participants