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

use Twitter::Extractor for creating links #2502

Merged
merged 1 commit into from May 5, 2017

Conversation

@masarakki
Copy link
Contributor

commented Apr 26, 2017

I found a problem in creating link tag.

Conditions:

  • Status contains mentions
  • Status contains url with =@username (in mentions)

For example,

@masarakki

http://example.com/?x=@masarakki

In this case,
At first, encode_and_link_urls convert url to <a href="http://example.com/?x=@masarakki" rel="nofollow ...">http://example.com/?x=@masarakki</a>.
Then, because of =@username matches to Account::ACCOUNT_RE, @masarakki will be convert to link by link_mentions.
At last, it will be <a href="http://example.com/?x=<a href="/...">@masarakki</a> rel="nofollow ...">

Sample: https://friends.nico/@masarakki/2717988

@Gargron

This comment has been minimized.

Copy link
Member

commented Apr 26, 2017

cc @abcang

@masarakki

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2017

I think simple replacement is dangerous, I have an idea to use placeholder-like replacement.

for example,

html, links = encode_and_link_urls("@masakki Hello, world!,  http://example.com/?x=@masarakki", [])
#=> html: "@masarakki Hello, world!, \1", links: ['<a href="http://example.com/?x=@masarakki">http://example.com/?x=@masarakki</a>']

html, links = link_mentions(html, link)
#=> html: "\2 Hello, world!, \1", links: ['<a href="http://example.com/?x=@masarakki>...</a>' '<a href="/users/1">@masarakki</a>']

html = build(html, links)
#=> '<a href="/usrs/1>@masarakki</a> Hello, world! <a href="http://example.com...'
....

of cource, \1 is not suitable because user can inject this string.

@abcang

This comment has been minimized.

Copy link
Collaborator

commented Apr 27, 2017

I think that it is safe to replace it after extracting the target like twitter-text.

@masarakki masarakki force-pushed the masarakki:over-linking branch from 977b233 to 977078b Apr 27, 2017
@masarakki masarakki changed the title over linking [WIP] over linking Apr 27, 2017
@masarakki masarakki changed the title [WIP] over linking [WIP] fix over linking Apr 27, 2017
@masarakki masarakki force-pushed the masarakki:over-linking branch 4 times, most recently from d3925ca to 4cfad36 Apr 27, 2017
@masarakki masarakki changed the title [WIP] fix over linking use Twitter::Extractor for creating links Apr 27, 2017
@masarakki masarakki force-pushed the masarakki:over-linking branch 9 times, most recently from 79ab3c0 to bd14cfc Apr 27, 2017
rel: 'nofollow noopener',
}
result = ''
def rewite(text, entities)

This comment has been minimized.

Copy link
@Gargron

Gargron May 2, 2017

Member

Is this meant to be called rewrite?

This comment has been minimized.

Copy link
@masarakki

masarakki May 5, 2017

Author Contributor

oh sorry

@masarakki masarakki force-pushed the masarakki:over-linking branch from e2378c9 to ae24c84 May 5, 2017
@Gargron
Gargron approved these changes May 5, 2017
@Gargron Gargron merged commit d08f111 into tootsuite:master May 5, 2017
2 checks passed
2 checks passed
codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@masarakki masarakki deleted the masarakki:over-linking branch May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.