Skip to content

Fix timestamp links in Youtube video descriptions#188

Merged
theScrabi merged 1 commit intoTeamNewPipe:devfrom
nyanpasu64:fix-description-timestamps
Aug 22, 2019
Merged

Fix timestamp links in Youtube video descriptions#188
theScrabi merged 1 commit intoTeamNewPipe:devfrom
nyanpasu64:fix-description-timestamps

Conversation

@nyanpasu64
Copy link
Contributor

@nyanpasu64 nyanpasu64 commented Aug 18, 2019

Fixes an issue where timestamps in video descriptions have their text and destination replaced with ...video?v=...#. This PR changes those links to point to ...video?v=...&t=timestamp but keeps the text unchanged. This causes NewPipe to play the video starting from the correct time.

This code handles both mm:ss and hh:mm:ss links (since :ss links are not made into clickable <a> by Youtube). Test case: https://www.youtube.com/watch?v=4cccfDXu1vA

For some reason, in NewPipeExtractor, comments were loaded from JSON by YoutubeCommentsInfoItemExtractor as text, sent via CommentsInfoItem#getCommentText to NewPipe, where timestamps are converted to hyperlinks using Linkify:
TeamNewPipe/NewPipe#2168

On the other hand, video descriptions are handled in NewPipeExtractor by scraping the watch-page HTML. There, timestamp links were previously mangled (and now properly parsed), before being sent as HTML via YoutubeStreamExtractor#getDescription to NewPipe (where HTML gets converted to Spanned).

The logic introduced in this commit is different from the above PR, since it operates in the extractor, and mutates the HTML DOM rather than identifying via regex. It makes more sense to put timestamp link conversion here, alongside converting other types of links, and to prevent the other if-statements here from mangling the links.

The code formatting and syntax may be unconventional (especially (onClickTimestamp = DESCRIPTION_TIMESTAMP_ONCLICK_REGEX.matcher(a.attr("onclick"))).find()). Maybe the variable names aren't the best.

No unit tests added yet :(

Fixes TeamNewPipe/NewPipe#1025.

  • I carefully read the contribution guidelines and agree to them.
  • I did test the API against NewPipe.
  • I agree to ASAP create a PULL request for NewPipe for making in compatible when I changed the api.

For some reason, in NewPipeExtractor,
comments were loaded from JSON by YoutubeCommentsInfoItemExtractor as text,
sent via CommentsInfoItem#getCommentText to NewPipe,
where timestamps are converted to hyperlinks using Linkify:
TeamNewPipe/NewPipe#2168

On the other hand, video descriptions are handled in NewPipeExtractor
by scraping the watch-page HTML.
There, timestamp links were previously mangled (and now properly parsed),
before being sent as HTML via YoutubeStreamExtractor#getDescription
to NewPipe (where HTML gets converted to Spanned).

The logic introduced in this commit is different from the above PR,
since it operates in the extractor, and mutates the HTML DOM
rather than identifying via regex.
@Stypox
Copy link
Member

Stypox commented Aug 18, 2019

This fixes TeamNewPipe/NewPipe#1025, doesn't it? Add Fixes TeamNewPipe/NewPipe#1025 to the pr description, so that GitHub closes that issue when this pr is merged :-)
Thank you for your work, this has been an issue for years :-D

@theScrabi theScrabi merged commit cfc72d8 into TeamNewPipe:dev Aug 22, 2019
@nyanpasu64
Copy link
Contributor Author

nyanpasu64 commented Aug 23, 2019

Thanks for merging!

Possibly you still need to merge TeamNewPipe/NewPipe#2536, otherwise videoDescriptionView.setAutoLinkMask(Linkify.WEB_URLS); will erase the URLs created here.

@nyanpasu64 nyanpasu64 deleted the fix-description-timestamps branch August 23, 2019 09:47
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.

[Feature Request] run the links of time of the description in the background player and video popup

3 participants