Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shorts #235

Merged
merged 4 commits into from
Jan 5, 2025
Merged

Fix shorts #235

merged 4 commits into from
Jan 5, 2025

Conversation

fire332
Copy link
Collaborator

@fire332 fire332 commented Dec 18, 2024

This PR is built on top of #234. That should be dealt with before this one. #234 is merged. Re-based onto main.

Fixes #210, fixes #211, fixes #219

Changes

  • Allow using TypeScript in app code by adding TypeScript support to babel. Cherry-picked into Misc dev changes #238.
  • Fix thumbnails in the shorts shelf.
  • Fix stretched/offset shorts player by changing the screensaver fix to not apply when not using the watch (full-screen player) page.
  • Disable "Remove Shorts from subscriptions" by default.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 19, 2024

Whoops. Had a brain fart when refactoring the screensaver fix.

@cremor
Copy link

cremor commented Dec 20, 2024

Thanks for working on this problem!
I've tested your changes (with a build based on #236) any I can indeed see the video of shorts again. But the video is not placed exactly in the correct area. There is a part of black space on the left side of the video area, and the right side of the video is cut off.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 20, 2024

@cremor can you take a picture?

@cremor
Copy link

cremor commented Dec 20, 2024

Sure. This is on an OLED C9 with WebOS 4.10.0:
Short

@fire332
Copy link
Collaborator Author

fire332 commented Dec 20, 2024

How did you take this picture? It looks more like the video is in the correct spot but the entire UI is offset

@cremor
Copy link

cremor commented Dec 20, 2024

It's just a photo I took with my phone. The short video was paused there, but that doesn't make a difference.
The thumbnail, which is visible for a moment before the short video playback starts, is not cut off. Only the actual video stream is.

@cremor
Copy link

cremor commented Dec 20, 2024

It looks more like the video is in the correct spot but the entire UI is offset

You are right. The white border isn't in the center of the screen. The video itself seems to be in the center.
But what could cause this? I never had this problem before the shorts video stopped being shown a few weeks/months back.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 20, 2024

Do you know how to use the web inspector tool? Can you check the computed styles of the top-most element that's offset incorrectly?

@cremor
Copy link

cremor commented Dec 20, 2024

No, but I think I could figure it out with a few hints. What software would I use to do that? I've only used WebOS Device Manager to install the app on the TV.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 20, 2024

You need node.js 22 LTS installed. Then download a copy of this repository and open the folder in terminal. Then run:

npm install
ares-inspect -a youtube.leanback.v4

@cremor
Copy link

cremor commented Dec 21, 2024

It was a bit more complicated than that 😅 I had to install the WebOS CLI tools and setup the device connection for my TV with it. Only then I could use the ares-inspect command.

But even then, how do I continue? I opened the shown "Application Debugging" URL in a browser and it showed YouTube under "Inspectable WebContents". But after clicking it I only get an empty page and the browser dev tools only show an empty body tag.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 21, 2024

Do you have the app launched on ur TV? If you don't, it's not going to work well.

@cremor
Copy link

cremor commented Dec 21, 2024

Yes, the app was running. I even had a short opened (in paused state).

@fire332
Copy link
Collaborator Author

fire332 commented Dec 21, 2024

It might be because you're using the CLI tools provided by LG. Try uninstalling it and use the CLI tools that are included by this project. Your terminal must be opened to the project directory and you need to prefix ares commands with npx for it to work.

@cremor
Copy link

cremor commented Dec 22, 2024

Same result.
But I noticed an error in the browser dev tools console: inspector.js:4031 Uncaught TypeError: document.registerElement is not a function

Seems like this is a deprecated function which is not supported in modern browsers any more: https://webostv.developer.lge.com/faq/2020-05-20-debugging-web-inspector-not-working-chrome-v80
I assume my TV (WebOS version) is too old. But TBH I don't really want to install that IDE or an old browser...

@fire332
Copy link
Collaborator Author

fire332 commented Dec 22, 2024

You can always try a portable version.

https://google-chrome-portable.en.uptodown.com/windows/download/2171677

@cremor
Copy link

cremor commented Dec 22, 2024

That "portable installer" doesn't work. It seems like that portable Chrome version isn't available any more (the error message contains a 404).

@cremor
Copy link

cremor commented Dec 22, 2024

Ok, I found a working version at https://portapps.io/app/ungoogled-chromium-portable/#download
Now I can see the HTML in the inspector.

This seems to be the top element of the border around the short video:
grafik

And here are its computed styles:
grafik

The computed styles of the video element might also be interesting:
grafik

@fire332
Copy link
Collaborator Author

fire332 commented Dec 22, 2024

Can you take a pic of your TV just like last time again but with the edges of your TV clearly visible?

@cremor
Copy link

cremor commented Dec 23, 2024

Sure:
Screen

@fire332
Copy link
Collaborator Author

fire332 commented Dec 23, 2024

Your html5-video-container dimensions and position matches up with mines so it's definitely the rest of the UI that's offset.

Can you show me the computed styles for the ytlr-player-focus-ring? You can find it easily by entering document.querySelector("ytlr-player-focus-ring") into the console, then right-click the result -> reveal in elements panel.

@cremor
Copy link

cremor commented Dec 23, 2024

Computed styles:
grafik

HTML context:
grafik

@fire332 fire332 marked this pull request as ready for review December 23, 2024 11:44
@fire332
Copy link
Collaborator Author

fire332 commented Dec 23, 2024

Show me the computed styles for the <ytlr-shorts-page> element right above it in your screenshot.

@fire332 fire332 added this to the v0.3.5 milestone Dec 23, 2024
@cremor
Copy link

cremor commented Dec 23, 2024

This is the last thing I can test with my TV until the holidays are over because I won't be home.

That element seems to cover the whole screen. But here are the computed styles anyway:
grafik

@fire332
Copy link
Collaborator Author

fire332 commented Dec 23, 2024

Your computed styles match up exactly with mines so the offset theoretically shouldn't be possible. Lemme know when you get back and feel like jumping on this problem again.

I appreciate your help. Happy holidays.

@victorrrz
Copy link

I tested this on a LG C1 (using webOS 6.4.0) and now shorts are working fine, with the previous version (v0.3.5-rc1) shorts looked bad, the video was out of frame and stretched horizontally.

@cremor
Copy link

cremor commented Dec 30, 2024

Nothing happens when I do this.
And I can even move the video frame back to the wrong position when I execute it with 0px (after executing it with 1px).

@drylmrls
Copy link

drylmrls commented Jan 2, 2025

I built my IPK from this PR on both [v0.3.5] and [v0.3.4] and shorts worked perfectly in my case on both versions. Thanks for this. I am using LG C2 42 inch (OLED42C2PSA).

By the way, would you know if its safe to update my TV to the latest version of WebOS if I want to retain this app? A new update just dropped and its a major one and big (1440 MB). It will be from 13.30.85 upgrading to 23.20.56.

@Trepd
Copy link

Trepd commented Jan 2, 2025

OLED55C6V
Built IPK from this PR and i have exact same problem as "cremor" in his pics.

Edit: typo

@fire332
Copy link
Collaborator Author

fire332 commented Jan 2, 2025

@cremor does 0.1px work?

@cremor
Copy link

cremor commented Jan 3, 2025

0.1px looks exactly the same as 1px.

If this additional border is indeed the fix I suggest to do it with black. That way it is nearly invisible.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 3, 2025

To be honest, it makes absolutely no sense how adding a border would correct the position of the video player. So I might just do the dirty border hack just for shorts to save my sanity.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 3, 2025

Can you try:

document.head.appendChild((function(){var x=document.createElement("style");x.innerHTML="* > video { transform: translateZ(0); }";return x;})())

@cremor
Copy link

cremor commented Jan 3, 2025

To be honest, it makes absolutely no sense how adding a border would correct the position of the video player.

Yeah, I don't understand it either. First I thought it's just the process of adding a style and that that somehow triggers a re-layout of the page. But the fact that applying the style with a 0px border reverts the effect contradicts that.

Can you try:

That doesn't seem to do anything.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 3, 2025

Try box-shadow: black 0px 0px; instead of the transform.

@cremor
Copy link

cremor commented Jan 3, 2025

Also does nothing.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 3, 2025

1px for both instead of 0px?

@cremor
Copy link

cremor commented Jan 3, 2025

Again nothing.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 3, 2025

Try:

document.head.appendChild((function(){var x=document.createElement("style");x.innerHTML=".WEB_PAGE_TYPE_SHORTS #html5-video-container { border-bottom: black 1px solid; border-image: url('data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7') 30%;}})())

@cremor
Copy link

cremor commented Jan 4, 2025

There is a syntax error with this. I assume you meant the following? But that also doesn't have any visible effect.

document.head.appendChild((function(){var x=document.createElement("style");x.innerHTML=".WEB_PAGE_TYPE_SHORTS #html5-video-container { border-bottom: black 1px solid; border-image: url('data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7') 30%;}";return x;})())

@fire332
Copy link
Collaborator Author

fire332 commented Jan 4, 2025

Yes I did. Replace the # with a . as well.

@cremor
Copy link

cremor commented Jan 4, 2025

Still no visible effect.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 4, 2025

Try with video instead of #html5-video-container.

@cremor
Copy link

cremor commented Jan 4, 2025

Like that? Still has no visible effect.

document.head.appendChild((function(){var x=document.createElement("style");x.innerHTML=".WEB_PAGE_TYPE_SHORTS video { border-bottom: black 1px solid; border-image: url('data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7') 30%;}";return x;})())

But the following works (moves the video):

document.head.appendChild((function(){var x=document.createElement("style");x.innerHTML=".WEB_PAGE_TYPE_SHORTS video { border: black 1px solid; border-image: url('data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7') 30%;}";return x;})())

I tested all 4 individual border sides, none work. Only the full border works.
Also it only works with video, but not with .html5-video-container.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 4, 2025

I tested all 4 individual border sides, none work. Only the full border works.

This makes absolutely no sense 😭. Thanks for testing.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 4, 2025

@cremor One final thing. Try:

document.head.appendChild((function(){var x=document.createElement("style");x.innerHTML=".WEB_PAGE_TYPE_SHORTS video { display: block;}";return x;})())

@cremor
Copy link

cremor commented Jan 4, 2025

No effect.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 5, 2025

Pushed the fix. Can you re-build on this branch again and see if it works?

@cremor
Copy link

cremor commented Jan 5, 2025

Doesn't work because you added border-bottom. If I change this to border then it works. I've tested this without dev tools, so I changed the css file and recompiled.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 5, 2025

Ahh fuck. I swear I used border. Don't code after a few drinks 🫠.

@fire332
Copy link
Collaborator Author

fire332 commented Jan 5, 2025

Try now.

@cremor
Copy link

cremor commented Jan 5, 2025

I can't see it directly because of the force-push, but you only changed border-bottom to border, right? Then I don't have to test it again, this is also what I changed and it worked then.

So in my opinion this can now be merged.

@fire332 fire332 merged commit 33f2f16 into webosbrew:main Jan 5, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants