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

white empty menu screen. satus.js bug? language files denied? #2160

Open
ImprovedTube opened this issue Apr 3, 2024 · 4 comments
Open

white empty menu screen. satus.js bug? language files denied? #2160

ImprovedTube opened this issue Apr 3, 2024 · 4 comments
Labels
help wanted Just an old github standard we add automatically. (The team can remove it when working on it.) Riddle

Comments

@ImprovedTube
Copy link
Member

ImprovedTube commented Apr 3, 2024

Similar has been around for years...
We load all settings every time the extension is clicked. Then chrome storage sometimes denies this for several seconds, resulting in: ...asking your browser what settings you made here before... :

youtube/menu/satus.js

Lines 891 to 912 in 01f8fa8

# IMPORT
--------------------------------------------------------------*/
satus.storage.import = function(keys, callback) {
var self = this;
if (typeof keys === 'function') {
callback = keys;
keys = undefined;
}
const overlay = document.createElement('div');
overlay.style.cssText = 'animation: fadeIn 4s linear; position: fixed; top: 50%; left: 50%; transform: translate(-50%, -50%); border: 3px solid rgba(182, 233, 255, 1); border-radius: 80px; padding: 37px; color: rgba(120, 147, 161, 1);';
overlay.textContent = '...asking your browser what settings you made here before...';
(document.body || document.documentElement).appendChild(overlay);
chrome.storage.local.get(keys || null, function(items) {
for (var key in items) {
self.data[key] = items[key];
}
// satus.log('STORAGE: data was successfully imported');
satus.events.trigger('storage-import');
if (callback) { callback(items); }
overlay.style.display = 'none';
});
};

- Didnt trace what else might causes the white screen.

Screenshot_655

( Reported after this change 🤔 430d410 @raszpl ) likely a coincidence.


We also process the whole language file each time

youtube/menu/satus.js

Lines 1036 to 1105 in 01f8fa8

satus.locale.import = function(code, callback, path) {
// if (!path) { path = '_locales/'; }
function importLocale(locale, successCallback) {
var url = chrome.runtime.getURL(path + locale + '/messages.json');
fetch(url)
.then(response => response.ok ? response.json() : {})
.then(data => {
for (var key in data) {
if (!satus.locale.data[key]) {
satus.locale.data[key] = data[key].message;
}
}
})
.catch(() => {})
.finally(() => successCallback && successCallback());
}
if (code) {
let language = code.replace('-', '_');
if (language.indexOf('_') !== -1) {
importLocale(language, () => importLocale(language.split('_')[0], () => importLocale('en', callback)));
} else {
importLocale(language, () => importLocale('en', callback));
}
} else { // try chrome://settings/languages:
try{chrome.i18n.getAcceptLanguages(function (languages) {
languages = languages.map(language => language.replace('-', '_'));
for (let i = languages.length - 1; i >= 0; i--) {
if (languages[i].includes('_')) {
let languageWithoutCountryCode = languages[i].substring(0, 2);
if (!languages.includes(languageWithoutCountryCode)) {
languages.splice(i + 1, 0, languageWithoutCountryCode);
}
}
}
languages.includes("en") || languages.push("en");
languages.forEach((language, index) => index === languages.length - 1 ? importLocale(language, callback) : importLocale(language, () => {}));
/* equals:
languages.length === 1 && importLocale(languages[0], callback);
languages.length === 2 && importLocale(languages[0], () => importLocale(languages[1], callback));
languages.length === 3 && importLocale(languages[0], () => importLocale(languages[1], () => importLocale(languages[2], callback)));
... */
// console.log(languages);
});} catch(error) {
// Finally, if code nor chrome://settings/languages are available, use window.navigator.language:
let language = window.navigator.language.replace('-', '_');
if (language.indexOf('_') !== -1) {
importLocale(language, () => importLocale(language.split('_')[0], () => importLocale('en', callback)));
} else {
importLocale(language, () => importLocale('en', callback));
}
console.log(error);
};
}
};
/*--------------------------------------------------------------
# TEXT
--------------------------------------------------------------*/
satus.text = function(element, value) {
if (value) {
if (satus.isFunction(value)) {
value = value();
}
element.appendChild(document.createTextNode(this.locale.get(value)));
}
};

The issue seemed to be less frequent with smaller files.
This should most-likely be a coincidence though. ( Even Crowdin also just made them a bit longer again, but just four tabulator keys & two line breaks per message.
crowdin
https://github.com/code-charity/youtube/pull/2039/files#diff-4127c9d4bf2969ed5053f8ecc759f3fb1239da11dac77cbd03ad11757e259c7e )

@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.) good first issue A GitHub standard for inviting (new) contributors *Congratulations in advance!* Completition / Revision Rethink, complete, improve, tweak our feature or structure. up-for-grabs (a github standard for inviting new contributors) - Welcome! ♥ Riddle labels Apr 3, 2024
@raszpl
Copy link
Contributor

raszpl commented Apr 4, 2024

I dont even understand what that user means :) "it whiteout white screen"? whole screen? like actual page goes white? extension popup screen? the second one displays a message so its not really all white

and no, for the hundreds time line breaks do not slow down loading files :--)

parsing all the settings and language files sounds horrible, but it is 2024
satus.locale.import() takes ~100ms for me to load locale
whole satus.storage.import() is ~400ms, and that includes above
and most of the time here is spend doing this :

youtube/menu/satus.js

Lines 899 to 902 in 7014a79

const overlay = document.createElement('div');
overlay.style.cssText = 'animation: fadeIn 4s linear; position: fixed; top: 50%; left: 50%; transform: translate(-50%, -50%); border: 3px solid rgba(182, 233, 255, 1); border-radius: 80px; padding: 37px; color: rgba(120, 147, 161, 1);';
overlay.textContent = '...asking your browser what settings you made here before...';
(document.body || document.documentElement).appendChild(overlay);

dynamic dom manipulations are really expensive, why is this created dynamically instead of being in index.html?
moving it to index.html reduces load time from ~400 to ~200ms
Still this is nothing compared to the time it takes to load all individual .js/CSS files, generate and render (browser repaint) satus-header/satus-main, and load expensive iframe from the web. Over 1 second for whole popup to show up.

just one change 200ms faster #2163
To test speed use
console.time('doSomething')
something to test
console.timeEnd('doSomething')
for example between lines 21 and 46

satus.storage.import(function (items) {

Then chrome storage sometimes denies this for several seconds

Iv never ever seen that happen yet.

@raszpl
Copy link
Contributor

raszpl commented Apr 4, 2024

ps I just did an experiment just for you ;) removed all EOL from messages.json and measured satus.storage.import() again, ZERO load time difference
The real cost is in importing 11 CSS and 18 .js files individually, and dynamically creating the menus. Ideally the first screen should be already pre generated in index.html. Well I tested the last thing and it makes no difference, guess waiting for menu to show up is just browser doing its thing, cant speed it up further at least on Vivaldi which is slow to begin with. Maybe it would make more impact in Firefox or Chrome. Ill let somebody else investigate.

@ImprovedTube ImprovedTube removed Bug Bug or required update after YouTube changes Completition / Revision Rethink, complete, improve, tweak our feature or structure. good first issue A GitHub standard for inviting (new) contributors *Congratulations in advance!* up-for-grabs (a github standard for inviting new contributors) - Welcome! ♥ labels Apr 4, 2024
@ImprovedTube
Copy link
Member Author

I meant a rare failure to show anything,
which might increased from pretty rare enough, to not rare enough.

I dont even understand what that user means :) "it whiteout white screen"? whole screen? like actual page goes white? extension popup screen? the second one displays a message so its not really all white

( since this got 4 votes. Only on that day, v4.820)

The issue seemed to be less frequent with smaller files.

and no, for the hundreds time line breaks do not slow down loading files :--)

yes, thats why it said

- Didnt trace what else might causes the white screen.

and #riddle: Dont know, if there is a bug /timing issue in satus.js, just seemed to noticed changes in frequency.
(While the recent changes look unlikely to have a relevant impact, i tend to add extra context and i didn't only want to mention your (nice) PR only, if any at all. It currently might loads 3 language files like pt_BR > pt > en, or more, if the user sets up many languages, which doesn't seem to make a difference either usually.)

ZERO

😬😳

The real cost is in importing 11 CSS and 18 .js files individually,

Yes, how much in average? amount of ms will vary by storage device & condition, - and if the extension was just updated, etc. ) They need not be split. Could combine them in one again(or else build with a build script.

@raszpl
Copy link
Contributor

raszpl commented Apr 6, 2024

if there is a bug /timing issue in satus.js

there sure are :o #1803 (comment)
indes.js should be executed last, and

youtube/menu/index.js

Lines 8 to 14 in 7014a79

/*--------------------------------------------------------------
# GLOBAL VARIABLE
--------------------------------------------------------------*/
var extension = {
skeleton: {}
};

moved to
/*--------------------------------------------------------------
# BASE
--------------------------------------------------------------*/
extension.skeleton = {
component: 'base'
};

changed #2163, this should solve any timing problems

combine them in one again(or else build with a build script.

best course of action for loading speed #1803 (comment)

ImprovedTube added a commit that referenced this issue May 6, 2024
Update satus.js correct Menu load timing problems #2160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Just an old github standard we add automatically. (The team can remove it when working on it.) Riddle
Projects
None yet
Development

No branches or pull requests

2 participants