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

home / front screen loading slow/incomplete (Audit please /rewriting our index.html. Etc.) #1803

Closed
Tracked by #1439
ImprovedTube opened this issue Oct 24, 2023 · 9 comments
Assignees
Labels
bounty Will pass on donations (Optional) - (OR: Requester will pay personally. Only if stated!) Bug Bug or required update after YouTube changes Completion / Revision Rethink, complete, improve, tweak our feature or structure. good first issue A GitHub standard for inviting (new) contributors *Congratulations in advance!* help wanted Just an old github standard we add automatically. (The team can remove it when working on it.) Knowledge Base / Dokumenation for developers We should repurpose this for future reference / Wiki / Education / Introduction Riddle up-for-grabs (a github standard for inviting new contributors) - Welcome! ♥

Comments

@ImprovedTube
Copy link
Member

ImprovedTube commented Oct 24, 2023

Hi, our index.html includes 30 tiny files (css & js) and sometimes loads blank / white for up to 10 seconds.. ( - thats only if you test on an old machine with busy ram / IO. Still this should not happen at all. The html and css can appear and be rendered instantly. If any JS runs, that can be afterwards) - While this issues might "only" cost less than 100 000 * 0.x seconds in human time per day (still a lot), it still looks bad as a first impressions and might hold back the whole project the most. (even if more importantly the original author paid a lot of attention to optimize code running on YouTube)


For reproducing the error you can move <script> tags back to the <body>
( I assume the original author had the script tags in the <body> section not to block half the page from appearing already, however this step wasn't finsihed, because once the <div class="satus-base is inserted already, in the middle of the <body, then the remaining scripts (=buttons) wont appear anymore (hence for now i moved them all to the <head>452b0dc )

youtube/menu/index.html

Lines 14 to 44 in 79de20d

<link rel="stylesheet" href="../js&css/satus.css">
<link rel="stylesheet" href="styles/analyzer.css">
<link rel="stylesheet" href="styles/appearance.css">
<link rel="stylesheet" href="styles/player.css">
<link rel="stylesheet" href="styles/blacklist.css">
<link rel="stylesheet" href="styles/fonts.css">
<link rel="stylesheet" href="styles/header.css">
<link rel="stylesheet" href="styles/home.css">
<link rel="stylesheet" href="styles/mixer.css">
<link rel="stylesheet" href="styles/night-mode.css">
<link rel="stylesheet" href="styles/search.css">
<link rel="stylesheet" href="styles/settings.css">
<link rel="stylesheet" href="styles/themes.css">
<script src="../js&css/satus.js"></script>
<script src="index.js"></script>
<script src="skeleton.js"></script>
<script src="skeleton-parts/search.js"></script>
<script src="skeleton-parts/active-features.js"></script>
<script src="skeleton-parts/settings.js"></script>
<script src="skeleton-parts/night-mode.js"></script>
<script src="skeleton-parts/mixer.js"></script>
<script src="skeleton-parts/general.js"></script>
<script src="skeleton-parts/appearance.js"></script>
<script src="skeleton-parts/themes.js"></script>
<script src="skeleton-parts/player.js"></script>
<script src="skeleton-parts/playlist.js"></script>
<script src="skeleton-parts/channel.js"></script>
<script src="skeleton-parts/shortcuts.js"></script>
<script src="skeleton-parts/blacklist.js"></script>
<script src="skeleton-parts/analyzer.js"></script>
<script src="functions.js"></script>


thanks!

@ImprovedTube ImprovedTube added Bug Bug or required update after YouTube changes help wanted Just an old github standard we add automatically. (The team can remove it when working on it.) important Critical or common. Thus to prioritize good first issue A GitHub standard for inviting (new) contributors *Congratulations in advance!* Structures (UX & ORG) Let's focus on structure! Everything should be as easily seen/found as it is relevant. Completion / Revision Rethink, complete, improve, tweak our feature or structure. up-for-grabs (a github standard for inviting new contributors) - Welcome! ♥ Knowledge Base / Dokumenation for developers We should repurpose this for future reference / Wiki / Education / Introduction labels Oct 24, 2023
@biyacraft
Copy link
Contributor

Hey, I can do this task

@ImprovedTube ImprovedTube added the bounty Will pass on donations (Optional) - (OR: Requester will pay personally. Only if stated!) label Nov 18, 2023
@ImprovedTube
Copy link
Member Author

hi! @Atesfahun awesome , looking forward! thanks for caring!
and sorry i missed the message yet.

(assignments aren't required or exclusive as it is still unlikely here that two people do the same thing at the same time)

@AbhinavGoel9
Copy link

@ImprovedTube Got it, I've identified the optimizations needed. I'll be working on async/defer loading for scripts, potential stylesheet merging, and exploring preloading critical resources to enhance page loading speed.

@ImprovedTube
Copy link
Member Author

thanks for caring @AbhinavGoel9! yes, we can have a build script for all CSS and JS to be in one script which should usually only save a few ms. not sure if am missing something.

@ImprovedTube
Copy link
Member Author

thanks @AbhinavGoel9! sorry, not sure it helps. (this was undone: https://github.com/code-charity/youtube/pull/1857/files )

this is the only alert i get from the scripts: code-charity/SATUS#10

@ImprovedTube ImprovedTube removed the important Critical or common. Thus to prioritize label Dec 19, 2023
@ImprovedTube ImprovedTube added Riddle and removed Structures (UX & ORG) Let's focus on structure! Everything should be as easily seen/found as it is relevant. labels Mar 11, 2024
@ImprovedTube
Copy link
Member Author

let me know if you see this again

@ImprovedTube ImprovedTube changed the title Audit please /rewriting our index.html. (Loading slow/incomplete). Etc. front screen loading slow/incomplete (Audit please /rewriting our index.html. Etc.) Mar 11, 2024
@ImprovedTube ImprovedTube changed the title front screen loading slow/incomplete (Audit please /rewriting our index.html. Etc.) home / front screen loading slow/incomplete (Audit please /rewriting our index.html. Etc.) Mar 11, 2024
@raszpl
Copy link
Contributor

raszpl commented Apr 6, 2024

For reproducing the error you can move <script> tags back to the

Cant reproduce. Extension popup loads fine with all scripts in head and all scripts in body. But if there would be a problem here, it would be the other way around. Moving scripts to head would introduce a bug (browser stops parsing while executing the script, can execute before body is created), and moving them to body would fix it.
Another way to fix it would be combining all scripts into one .js file and loading it in head with async attribute.

The main thing that happens when you move scripts to head is you make the browser parse and execute them serially one after another before whole html document is loaded and interpreted. This means that

<script src="satus.js"></script>
<script src="index.js"></script>

is being executed before document.body exists at all. This means its possible for
satus.render(extension.skeleton);

thus
this.append(element, container);

thus

youtube/menu/satus.js

Lines 267 to 268 in 7014a79

satus.append = function(child, parent) {
(parent || document.body).appendChild(child);

to also execute before document.body has been created and crash. This will be non deterministic and depend on system speed and browser used.
Hell, now that Im looking at it its probably even possible for

satus.render(extension.skeleton);

to execute before all the other .js files loaded with contents of the menu - this could result in partially loaded menus.

( I assume the original author had the script tags in the section not to block half the page from appearing already,

keeping scripts in body ensured document.body already existed when executing satus.render()

however this step wasn't finsihed, because once the <div class="satus-base is inserted already, in the middle of the <body, then the remaining scripts (=buttons) wont appear anymore (hence for now i moved them all to the

Can you trigger this error, right click on the resulting non working extension popup, open devtools and copy whats in the console.log?

@raszpl
Copy link
Contributor

raszpl commented Apr 6, 2024

Another way to fix it would be combining all scripts into one .js file and loading it in head with async attribute.

This is the best thing that can be done for fast nd reliable loading with no weird timing issues. Combining can be done during the build process, so source code in github looks just like now with everything separated into its own files, but extension ships with only one file called index.html containing <script> tag with concatenated contents of all of those:
functions.js index.js satus.js skeleton.js active-features.js analyzer.js appearance.js blocklist.js channel.js general.js mixer.js night-mode.js player.js playlist.js search.js settings.js shortcuts.js themes.js

@raszpl
Copy link
Contributor

raszpl commented Apr 6, 2024

for example this will never have timing problems and will always load reliably:
menu.zip

all .css bundled in html, all .js bundled into one file. Optimally .js would be inlined, but that requires signed file in manifest so I didnt bother.

Trivial build script:

type index1.html > index.html

type satus.css >> index.html
type styles\home.css >> index.html
type styles\appearance.css >> index.html
type styles\search.css >> index.html
type styles\header.css >> index.html
type styles\sub-options.css >> index.html
type styles\analyzer.css >> index.html
type styles\blocklist.css >> index.html
type styles\fonts.css >> index.html
type styles\mixer.css >> index.html
type styles\settings.css >> index.html
type styles\themes.css >> index.html

type index2.html >> index.html

type satus.js > all.js
type skeleton.js >> all.js
type functions.js >> all.js
type skeleton-parts\search.js >> all.js
type skeleton-parts\active-features.js >> all.js
type skeleton-parts\settings.js >> all.js
type skeleton-parts\night-mode.js >> all.js
type skeleton-parts\mixer.js >> all.js
type skeleton-parts\general.js >> all.js
type skeleton-parts\player.js >> all.js
type skeleton-parts\appearance.js >> all.js
type skeleton-parts\shortcuts.js >> all.js
type skeleton-parts\channel.js >> all.js
type skeleton-parts\playlist.js >> all.js
type skeleton-parts\themes.js >> all.js
type skeleton-parts\blocklist.js >> all.js
type skeleton-parts\analyzer.js >> all.js
type index.js >> all.js

just requires index.html to be divided into two parts index1.html

<!doctype html><html><head><meta charset="utf-8"><meta name="viewport" content="width=device-width">
<title>ImprovedTube</title><style>body{width:320px;max-width:320px; margin:0; height:600px;max-height:600px;"}</style>
<style type="text/css">

index2.html

</style>
<script defer src="all.js"></script>
</head><body></body></html>

Also there is no need for "...asking your browser what settings you made here before..." to ever be present because there will never be any delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty Will pass on donations (Optional) - (OR: Requester will pay personally. Only if stated!) Bug Bug or required update after YouTube changes Completion / Revision Rethink, complete, improve, tweak our feature or structure. good first issue A GitHub standard for inviting (new) contributors *Congratulations in advance!* help wanted Just an old github standard we add automatically. (The team can remove it when working on it.) Knowledge Base / Dokumenation for developers We should repurpose this for future reference / Wiki / Education / Introduction Riddle up-for-grabs (a github standard for inviting new contributors) - Welcome! ♥
Projects
None yet
Development

No branches or pull requests

4 participants