Catch errors while processing timestamp-links

[litetex]

What is it?

• Bugfix (user facing)
• Feature (user facing)
• Codebase improvement (dev facing)
• Meta improvement to the project (dev facing)

Description of the changes in your PR

Catches errors while processing timestamp-links.

Fixes the following issue(s)

• Fixes Clicking the 1st comment of this youtube video to expand exists & crashes newpipe to homescreen #6842

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

• I read the contribution guidelines.

[TobiGr]

Why is that method even called? This fixes the symtoms of parsing an unexpactedly structured URL. Shouldn't we take a closer look at the PATTERN?

(\\d+:)?(\\d+)?:(\\d+)

This regex looks like it is accepting more than it should.

[Stypox]

Maybe we should use:

(\\d{1,2}:)?(\\d{1,2})?:(\\d{1,2})

[litetex]

I think a problem with using

(\\d{1,2}:)?(\\d{1,2})?:(\\d{1,2})

Is that you get cut-off results like:

Should we stick with the old implementation?

Btw: Is it correct that even results like :12 are consider valid?

[Stypox]

Is that you get cut-off results like:

Oh ok, then nevermind. Just keep the current one, but check that minutes and seconds are <= 60

Btw: Is it correct that even results like :12 are consider valid?

I think so, but ask @ TiA4f8R Edit: according to the regex in TextLinkifier it's wrong.

[Stypox]

Wait, but why isn't TextLinkifier used? There the correct timestamps regex is used (we worked on it together a couple hours on Element):

(?:^|(?!:)\W)(?:([0-5]?[0-9]):)?([0-5]?[0-9]):([0-5][0-9])(?=$|(?!:)\W)

[Stypox]

Using TextLinkifier.TIMESTAMPS_PATTERN and this code for timestampLink seems to work for me. Using TextLinkifier wouldn't work since it would disable clicks on the text view.

    private final Linkify.TransformFilter timestampLink = new Linkify.TransformFilter() {
        @Override
        public String transformUrl(final Matcher match, final String url) {
            final int timestampStart = match.start(2);
            final int timestampEnd = match.end(3);
            final String parsedTimestamp = commentText.substring(timestampStart, timestampEnd);
            final String[] timestampParts = parsedTimestamp.split(":");

            final int seconds;
            if (timestampParts.length == 3) { // timestamp format: XX:XX:XX
                seconds = Integer.parseInt(timestampParts[0]) * 3600 // hours
                        + Integer.parseInt(timestampParts[1]) * 60 // minutes
                        + Integer.parseInt(timestampParts[2]); // seconds
            } else if (timestampParts.length == 2) { // timestamp format: XX:XX
                seconds = Integer.parseInt(timestampParts[0]) * 60 // minutes
                        + Integer.parseInt(timestampParts[1]); // seconds
            } else {
                return url;
            }
            return streamUrl + url.replace(match.group(0), "#timestamp=" + seconds);
        }
    };

[litetex]

I'm also currently writing some unit-tests and fun fact:
"Timestamp format: XX:XX:XX" never works because the wrong group (it should be group 1 but it's group 2) is matched 😆

So e.g. 1:23:45 becomes 1425s (23:45) when it should be 5025s

Fixing it on the fly

[Stypox]

Thank you, I like this implementation :-)

[opusforlife2]

@litetex Remember to always add a PR to the draft release whenever you merge it.