:bug: Add delay to SetFocus for the 'Similar' sidemeu icon in the context menu to avoid triggering auto trailer playback in Spotlight [Proposed Fix]

OfficerKD637 commented 2 weeks ago

Skin section

Widgets

Current Behavior

Yup it's me again haha! I am currently tweaking the skin to get the Spotlight to navigate like the 'Fullscreen Widget' in AH2 and keep finding bugs (and their fixes!). I swear I'm not doing this on purpose.

Clicking on the context menu button in the Spotlight and then clicking on the 'Similar' side menu button will pop up the new side dialog but still keeps the context menu button focused while you're scrolling through the menu. Thus leading to the timer for the Auto Trailer On Focus to run out and the trailer starts playing.

Adding a Simple AlarmClock here to delay the SetFocus solved the issue for me.

<onclick>AlarmClock(test,Control.SetFocus(50),00:01,silent)</onclick>

This is the simplest way I could think of to avoid triggering the auto trailer playback.

Let me know what you think!

Expected Behavior

Move the focus from the Spotlight context menu button after clicking on the 'Similar' side menu button to avoid triggering the auto trailer playback

Steps To Reproduce

Enable auto trailer playback in Spotlight
Click on the spotlight context menu button and subsequently the 'Similar' side menu button
Wait for 5 seconds

Screenshots and Additional Info

No response

Checklist

[X] I have made a separate issue for each bug that I am reporting.
[X] I have searched the issues and this bug has not been reported.
[X] I am using the most recent skin version.
[X] I have confirmed that this bug does not occur in the default skin.

jurialmunkey commented 2 weeks ago

I think the more correct and simpler approach is simply to cancel the playtrailer alarmclock when clicking the context, info, or play/browse buttons. No need to try to hack it by messing with contextmenu and adding workarounds all over the place.

OfficerKD637 commented 2 weeks ago

I think the more correct and simpler approach is simply to cancel the playtrailer alarmclock when clicking the context, info, or play/browse buttons. No need to try to hack it by messing with contextmenu and adding workarounds all over the place.

I tried that as well, but after clicking on the 'Similar' side button (fire icon) beside the context menu it seems Kodi doesn't remove the focus from the 'Context' button. Somehow it looks like there is double focus (i.e your current control focus (id=1001) that's scrolling the list and a different one behind the dialog (id=313) on the actual context button).

I think this is what's triggering the AutoTrailer_OnFocus action?

Here's a screenshot to better demonstrate it:

Click on the context menu button in the Spotlight
Click on the 'Similar' side menu button

Two highlighted objects (probably indicating two focused items simultaneously?)

Focus issue

I tried circumventing this by making sure the focus doesn't stay on the context button after clicking on the 'Similar' side button by adding an AlarmClock. This doesn't break the context menu elsewhere in the skin (I think)

This is all speculation btw, and the issue could be something else entirely. Please let me know what you think.

jurialmunkey commented 2 weeks ago

Ah yeah I see. Kodi is refocusing the control when the contextmenu closes, which is retriggering the trailer alarmclock (onfocus of button)

The SetFocus(50) command is actually completely irrelevant here. It isn't causing the refocus (that happens with or without it). In fact, I'm pretty sure the SetFocus isn't actually needed at all and is just a relic of an old method I didn't cleanup properly.

The only reason delaying that SetFocus command with an AlarmClock "works" is because it leaves enough time for the ContextMenu to close first (400ms animation) and allow the trailer alarm to be restarted first (onfocus of menu button after animation). Then the SetFocus is firing after that occurs, so focus moves away from the button again, which triggers the onunfocus command (which then cancels the trailer alarm).

Basically, you'd achieve exactly the same thing by replacing SetFocus(50) with: AlarmClock(LetsHaveARaceToCancelTheAlarm,CancelAlarm(playtrailer,silent),00:01,silent)

Which is a bit more "correct" but still not great because we're essentially racing the alarm against the animation speed of the system and hoping the commands fall in the right order.

I think the only good way to do any of this is a complete overhaul of how trailers are managed by switching them to use Timers.xml instead.

jurialmunkey commented 2 weeks ago

In the meantime though, we can just cancel the alarm when the contextmenu opens. That way we avoid layering on additional workarounds which might have later unintended effects (plus I'm pretty likely to forget why the delay was added, which means I'll just remove it again and we're in the same spot!).

I'll also add in a cancelalarm onclick because that's just good practice anyway rather than only relying on onunfocus to cancel it.

jurialmunkey / skin.arctic.fuse