Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement stripping of s and t parameters from Twitter links. #40

Merged
merged 1 commit into from
Jul 30, 2022

Conversation

demize
Copy link
Contributor

@demize demize commented Jul 26, 2022

Should address #37, though I didn't make it configurable as suggested there.

Also added a test URL to test_urls.md and test_urls.html.

Did not bump the version in the manifest; I can do that if necessary, though I figured I'd leave it up to the maintainers to decide when to bump the version.

@apiraino
Copy link
Owner

@demize looks great, thanks!

Minor nit: next time please split code formatting changes in a separate commit, it makes reviewing the actual code changes a little bit easier.

@apiraino apiraino merged commit a1e189f into apiraino:master Jul 30, 2022
@apiraino
Copy link
Owner

closes #37

@ZacharyTalis
Copy link

Does this PR implement cleaning for Copy link to Tweet events? If so, I'm not able to replicate. Cleaning Twitter URLs in the address bar works as expected o7

Specs: Firefox Nightly v104 on Linux w/ web-ext-run.sh environment

@demize
Copy link
Contributor Author

demize commented Aug 9, 2022

Does this PR implement cleaning for Copy link to Tweet events? If so, I'm not able to replicate. Cleaning Twitter URLs in the address bar works as expected o7

Specs: Firefox Nightly v104 on Linux w/ web-ext-run.sh environment

That's my bad; it looks like the cleaners need to be specified separately when the menu item is handled, and I missed that in my initial review. I can create a new PR to fix that tonight.

@ZacharyTalis
Copy link

thank youuu <3

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.

3 participants