Skip to content

Conversation

@orbachar
Copy link

Problem

Twitter/X.com URLs with query parameters or path segments after the status ID were not being recognized by the embed regex pattern.

Examples of URLs that didn't work:

  • https://x.com/cb_doge/status/1817235892916146413/photo/1
  • https://x.com/MrShibolet/status/1990842843481387410?s=20

Solution

Updated the Twitter service regex pattern to properly handle:

  1. Query parameters (e.g., ?s=20, ?s=20&t=xyz)
  2. Path segments after the status ID (e.g., /photo/1)

Changed regex from:

/^https?:\/\/(www\.)?(?:twitter\.com|x\.com)\/.+\/status\/(\d+)/

To:

/^https?:\/\/(www\.)?(?:twitter\.com|x\.com)\/.+\/status\/(\d+)(?:\?.*)?$/

The new pattern uses (?:\?.*)?$ to optionally match query parameters and ensures the pattern matches to the end of the string, which allows for any path segments or query parameters after the status ID.

Testing

Added comprehensive test cases including:

  • URLs with query parameters
  • URLs with path segments (/photo/1)
  • Both twitter.com and x.com domains

All existing tests continue to pass.

- Updated regex to properly handle URLs with query parameters (e.g., ?s=20)
- Updated regex to properly handle URLs with path segments after status ID (e.g., /photo/1)
- Added test cases for x.com URLs with query parameters and path segments
- Fixes issue where https://x.com URLs with additional parameters were not recognized
The previous regex only handled query parameters but not path segments.
Updated to: /^https?:\/\/(www\.)?(?:twitter\.com|x\.com)\/.+\/status\/(\d+)(?:\/.*)?(?:\?.*)?$/

This now properly matches:
- https://x.com/cb_doge/status/1817235892916146413/photo/1
- https://x.com/status/123?s=20
- https://x.com/status/123/photo/1?s=20
@orbachar
Copy link
Author

Fixes:
#129

Copy link
Contributor

@neSpecc neSpecc left a comment

Choose a reason for hiding this comment

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

Please, attach an issue correctly. And increment a patch version

@neSpecc
Copy link
Contributor

neSpecc commented Nov 21, 2025

Please, update branch from master. I've fixed CI in there.

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.

2 participants