Player Size: full window with theater mode hides player after search

[sarim]

⚬ BUG:
If you click on youtube search box, Then click outside somewhere, video player gets hidden.

⚬ HOW:

1. From extension option set Player Size: Full Window, Forced Teather Mode
2. Open a youtube video.
3. Click on youtube search box, maybe type something. DO Not click on any result.
4. Scroll down, click on a empty spot (NOT on any link) somewhere, in comment section or sidebar etc...

⚬ EXPECTED (/preferred) behavior:
Video keep showing.

⚬ Screenshots:
Screencap:

Screenshot.2023-02-01.04.38.15.mp4

Setup:

⚬ ImprovedTube Version: 4.0.17
⚬ Browser: Google Chrome 109.0.5414.120
⚬ Settings:

{"forced_theater_mode":true,"player_size":"full_window"}

⚬ OS: Windows 11 22H2 OS Build 22621.1194
⚬ Device: Desktop Computer

[dojoVader]

When the it-header-position is set to null It breaks the video, but changing the value manually fixes it.

[sarim]

@dojoVader Can you elaborate please, changing to what value? It's always null for me, setting it to 10px / 20px etc.. doesn't solve.

[dojoVader]

@sarim apologies for not explaining

So I debugged it when you focus out, it tries to read a property from DataSet but it comes back with a null, when a null is set, this hides the player, Here is a screenshot for more clarity.

Source: general.js
Line: 550

[dojoVader]

@sarim I encountered the same issue, but when I disabled and enabled it to replicate it, I didn't get the issue again, can you confirm this on your side,

I'm trying to replicate the issue, to trace what's setting it as null.

[sutamouli12]

Thank you

…

On Tuesday, 7 February 2023, Okeowo Aderemi ***@***.***> wrote: @sarim <https://github.com/sarim> apologies for not explaining So I debugged it when you focus out, it tries to read a property from DataSet but it comes back with a null, when a null is set, this hides the player, Here is a screenshot for more clarity. Source: general.js Line: 550 [image: image] <https://user-images.githubusercontent.com/789578/217210086-2fa79515-5c88-4306-832f-8cd3a70c3529.png> — Reply to this email directly, view it on GitHub <#1555 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A37QRWDLL3UI3CFFB2ELV4TWWIK6ZANCNFSM6AAAAAAUM766BM> . You are receiving this because you are subscribed to this thread.Message ID: <code-for-charity/ImprovedTube-for-YouTube/issues/1555/1420483617@ github.com>

[sarim]

@dojoVader ah you're running it from source? I didn't dig that deeper.

I encountered the same issue, but when I disabled and enabled it to replicate it, I didn't get the issue again, can you confirm this on your side,

Issue still present on my side.

[dojoVader]

@sarim I am going to run the extension from the store and confirm it today.

[ImprovedTube]

@dojoVader currently the same

[dojoVader]

@ImprovedTube @sarim Thanks click full_window causes the bug.

[dojoVader]

@ImprovedTube it-position-header-original contains a null, that null is used to set the it-position-header, which causes the player to disappear, rather than null Do we default to a "normal" or "static" ?

Here is a video with a fix applied, let me know what you think

https://www.awesomescreenshot.com/video/14622583?key=30182d888bc763c9b7ca072aa99b3e95

[dojoVader]

@ImprovedTube waiting for feedback, this code shows why it's failing so I am asking if we switch the default value from null to either "normal" or "static".

[sarim]

@dojoVader Another question. I'm not telling improvetube to change header style, then why the function is getting called? I think the function shouldn't even be called if there is no setting set in appearance->header section.

[ImprovedTube]

thank you guys! please make a pull request anytime, for any fix or quick fix.

The bigger picture:

Player Size: Full window

"Full window" sets header size 0px.

• But why does the bug even happen?

function shouldn't even be called if there is no setting set in appearance->header

Yes! conditions for this function to run:

• only when our header-hiding also runs triggered by one one of our header options (except "static")

• And we can also avoid even running an extra eventListeners by running it as a sub-routine of the ShortcutGoToSearch here https://github.com/code-for-charity/ImprovedTube-for-YouTube/blob/df240abc56b8761885b3ff74c1a10100475bd32a/content-scripts/website-context/youtube-features/shortcuts.js#L540

  • Besides you wanna check how-when-why-if our code is overwriting Youtube's default shortcuts? '/' is youtubes default. (We can try to adjust/remap - or remove Youtube's eventListeners too ( filtering the http answer, before the the page even runs,.... ) (Remap Youtube Shortcuts? Keep visual feedback. (Display default YouTube icons and overwrite default shortcuts) #110)

BTW: Ultimately we can handle all input & shortcuts at once: #1565

---

default

#1685

[sarim]

For quick fix I added this with a tampermonkey script

document.querySelector("html").setAttribute("it-header-position", 'normal');

This seems to solve the problem for now.

[ImprovedTube]

exactly. While we should also check Player Size:Full Window code or youtube DOM to investigate why it happens

aand the quick-fix for us to upload asap can be @dojoVader's or better wrapping it in another

if (headerPos) {

https://github.com/code-for-charity/ImprovedTube-for-YouTube/blob/bf7c3bc21a1848d75d44a0211e1d58e167b79f47/content-scripts/extension-context/youtube-features/general/general.js#L565-L576

}

(+ exchanging line 564 & 565 for a little optimisation.. https://github.com/code-for-charity/ImprovedTube-for-YouTube/blob/bf7c3bc21a1848d75d44a0211e1d58e167b79f47/content-scripts/extension-context/youtube-features/general/general.js#L564 )

[ImprovedTube]

labeling this "up for grabs" since the Pull Request for the bug only requires copy paste anymore at this point.

"feature request", "completion" & "Lets focus on structure" are about the 8 tasks I defined above, which shall become new issues-threads if you will