Skip to content
This repository has been archived by the owner on Aug 18, 2023. It is now read-only.

Remove t.co redirection even on profile website link #590

Merged
merged 2 commits into from Apr 18, 2021

Conversation

hiroto7
Copy link
Contributor

@hiroto7 hiroto7 commented Apr 14, 2021

Fixes #416.

Since the method createUrlAnchor() is not called when rendering the profile website link, I added template modification.

Copy link
Owner

@eramdam eramdam left a comment

Choose a reason for hiding this comment

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

Thanks for this 🙇‍♂️ Overall looks good, just left a small comment

@@ -29,4 +30,16 @@ export const maybeRemoveRedirection = makeBTDModule(({TD, settings}) => {

return result;
};

TD.services.TwitterUser.prototype.getExpandedURL = function () {
return this.entities.url.urls[0].expanded_url;
Copy link
Owner

Choose a reason for hiding this comment

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

Could we add some checks here? I feel like this might crash if a given user doesn't have an url?

Copy link
Contributor Author

@hiroto7 hiroto7 Apr 16, 2021

Choose a reason for hiding this comment

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

As far as the code in this PR, getExpandedURL() is not called for users who do not have a website link so that no crash will occur.
However, with this, templates such as {{#getExpandedURL}} ... {{/getExpandedURL}} cause crash, now I think we should add an existence check too.

So, I'll add the check similar to TD.services.TwitterUser.prototype.getDisplayURL (implemented by original TweetDeck)!

Copy link
Owner

Choose a reason for hiding this comment

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

@hiroto7 sounds good! I prefer to be extra careful because crashing in there can have weird consequences 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it in 84ead19.

@eramdam eramdam merged commit ad0e9dc into eramdam:main Apr 18, 2021
@hiroto7 hiroto7 deleted the RemoveRedirection branch April 19, 2021 05:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Remove t.co redirection on links" not working with profile website link
2 participants