Enable TV Support for 'Ask to Skip'

[viown]

Changes
Enables TV Support for the skip button

Issues

[jellyfin-bot]

Cloudflare Pages deployment

Latest commit	5f10569
Status	✅ Deployed!
Preview URL	https://de6e4d52.jellyfin-web.pages.dev
Type	🔀 Preview

View build logs

[dmitrylyzo]

1. I think we should focus the skip button (on show) if OSD is hidden.
2. Found bug: _skipElement is not reattached on the second video playback. Separate PR.
   1. Start video playback.
   2. Wait for the Skip button to appear.
   3. Stop playback (return to the library/item).
   4. Resume video playback. Button is not attached to the DOM.
3. We should use emby-button to highlight the focus state. There is no (native) focus indication in webOS 1.2.

[viown]

I think we should focus the skip button (on show) if OSD is hidden.

I agree, it's not intuitive to have to click up two times when the button appears.

Found bug: _skipElement is not reattached on the second video playback. Separate PR.

Thanks! This is an issue with just this PR because videoOsdPage is unmounted when playback stops and it originally only created the button once in the body and re-used it on second playback.

[dmitrylyzo]

@viown Could you try changing the skip button to emby-button?

We should use emby-button to highlight the focus state. There is no (native) focus indication in webOS 1.2.

Alternatively, we can add focus and hover (#6364) styles. But emby-button is already styled.

Also, don't focus the skip button when the OSD is visible.

[viown]

Yes, to do this I will have to move the button to the OSD template, which is probably what I should've done to begin with.

Also, the button should not focus when the OSD is visible. Do you see different results in your testing?

[dmitrylyzo]

Do you see different results in your testing?

Yes, it grabs the focus when I navigate the playback control buttons (to keep the OSD visible).

With this clip: https://github.com/user-attachments/assets/95ac625f-a41d-46d1-9125-1f6c4356552a

1. Switch to the TV display mode.
2. Enable Ask to Skip for Intro and Outro only.
3. Start that clip.
4. Seek to the middle.
5. Use the Left/Right buttons to move between the playback control buttons (to keep OSD visible).
6. When `Outro' begins, the skip button grabs the focus.

[viown]

Updated to use emby-button. Your mentioned issue should be fixed. I also noticed that the button would cause the video to pause on click (just this PR), so that was fixed as well.

Given how complex this PR became, I wonder if it still makes sense for a 10.10.z release?

[dmitrylyzo]

1. this.skipElement seems to become invalid when the next episode starts playing.
   Start playlist with 2 video items with chapters.
   Wait for the skip button to appear.
   Click Next track.
   The skip button doesn't appear unless you refresh the web page.

2. This code pauses playback when using Enter on the skip button (in the TV mode).
   As a quick fix:

if (e.target.tagName !== 'BUTTON') {
    playbackManager.playPause(currentPlayer);
}

We are losing the ability to pause using the `OK' button on the TV remote control, so we need to rethink the skip button focus logic and/or UI.

Given how complex this PR became, I wonder if it still makes sense for a 10.10.z release?

Alternatively, we could try moving the skip button back from the video OSD and block the focus change by checking the current document.activeElement. Don't change the focus if activeElement is a valid focusable element.
The problem is that activeElement can be undefined, body or the last focused element when out of focus when hiding an element. So it should be checked with care, but isCurrentlyFocusable is probably sufficient.

[dmitrylyzo]

A few minor notes. Not tested yet.

[dmitrylyzo]

Almost fine.

1. There's a problem with CustomElements (see below). Major issue.
2. The focus is lost after we hide the skip button.
   This is a global problem. If we don't find a good concept for changing focus after hiding the current element, I think we can leave it to the next version - the user can use Back to hide the OSD and show it again using the navigation keys.

[viown]

Yeah, I noticed that focus was lost after clicking the button, but wasn't sure what the best way to go about it was.

Since skipping will summon the OSD panel, we might be able to change the focus to the play/pause button. Adding focusManager.focus(document.body.querySelector('.btnPause')); after this line seems to work (with checks for TV layout).

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[dmitrylyzo]

Not sure what to do with focus (focus loss can happen not only on the video OSD), but we can probably fix that later. As workaround: users can hide the OSD with Back/Escape buttons and regain focus when it reappears.

[rlauuzo]

To address the focus issue, you can make the following changes:

1. Update lines 1254-1256 in index.js to handle the Enter key differently based on the element triggering the event:

   case 'Enter':
       showOsd(e.target.classList.contains('skip-button') ? btnPlayPause : undefined);
       break;

2. Prevent the skip button from losing focus by modifying lines 357-359:

   if (document.activeElement && !document.activeElement.classList.contains('skip-button')) {
       document.activeElement.blur();
   }

[dmitrylyzo]

1. Update lines 1254-1256 in index.js to handle the Enter key differently based on the element triggering the event:

I think it would be better to restore it here:

jellyfin-web/src/controllers/playback/video/index.js

Lines 341 to 343 in 5f10569

	} else if (currentVisibleMenu === 'osd' && focusElement && !layoutManager.mobile) {
	_focus(focusElement);
	}

We need to remove focusElement from the condition. If !focusElement and document.activeElement is not focusable, use btnPause as the focus element.

 } else if (currentVisibleMenu === 'osd' && !layoutManager.mobile) {
    if (!focusElement && (!document.activeElement || !focusManager.isCurrentlyFocusable(document.activeElement)) {
        focusElement = osdBottomElement.querySelector('.btnPause');
    }
    _focus(focusElement); 
 }

Not tested.

Iirc, we also need to add range input (playback progress bar) as focusable element to isCurrentlyFocusable. The skip button grabs focus from it.

2. Prevent the skip button from losing focus by modifying lines 357-359:

We should check that document.activeElement is a child of osdBottomElement.