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

Update core.js limit localstorage writes to only essential ones #2180

Merged
merged 1 commit into from
Apr 21, 2024
Merged

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Apr 10, 2024

currently we are writing on every page load, and its my fault :(

currently we are writing on every page load, and its my fault :(
@ImprovedTube
Copy link
Member

ImprovedTube commented Apr 11, 2024

hi! @raszpl this renews localStorage, if it was lost and makes the feature work again the next time. (So it need not be before improvedTube.init() either.

(Either way the feature can run async.)


btw, loading the storage object could be faster chrome.storage.local.get(null), before processing/setting the <html>-attributes one by one at

extension.storage.load = function (callback) {
chrome.storage.local.get(function (items) {
for (var key in items) {
var value = items[key];
extension.storage.data[key] = value;
document.documentElement.setAttribute('it-' + key.replace(/_/g, '-'), value);
}
extension.events.trigger('storage-loaded');
extension.messages.send({
action: 'storage-loaded',
storage: items
});
if (callback) {
callback(extension.storage.data);
}
});
};

@raszpl
Copy link
Contributor Author

raszpl commented Apr 11, 2024

hi! @raszpl this renews localStorage, if it was lost and makes the feature work again the next time. (So it need not be before improvedTube.init() either.

I know, I wrote this code :P c725cfd
but I did it poorly, this patch makes it write less.

  • And instead of local storage, the lines after L172 add a combined variable to chrome.local.storage, called HTMLMediaElementSettingsCodecFpsEtc
    So that the feature (L72-L101) can receive that from content scripts or run directly in a content script. chrome.storage.local.get('HTMLMediaElementSettingsCodecFpsEtc', function (result) { ...

are you commenting on some internal version or is this a wishlist? :)
cant use chrome.storage.local.get in content script injected into YT, its only available in private side of extension
cant override window.MediaSource.isTypeSupported from extension/core.js its a wrong execution context
we would have to inject javascript tag here to override MediaSource

btw, loading the storage object could be faster chrome.storage.local.get(null), before processing/setting the <html>-attributes one by one at

extension.storage.load = function (callback) {
chrome.storage.local.get(function (items) {
for (var key in items) {
var value = items[key];
extension.storage.data[key] = value;
document.documentElement.setAttribute('it-' + key.replace(/_/g, '-'), value);
}
extension.events.trigger('storage-loaded');
extension.messages.send({
action: 'storage-loaded',
storage: items
});
if (callback) {
callback(extension.storage.data);
}
});
};

I mean, sure

extension.storage.load = function (callback) {
	chrome.storage.local.get(function (items) {
		extension.storage.data = items;

		extension.events.trigger('storage-loaded');
		extension.messages.send({
			action: 'storage-loaded',
			storage: items
		});

		for (const key in items) {
			document.documentElement.setAttribute('it-' + key.replace(/_/g, '-'), items[key]);
		}

		if (callback) {
			callback(extension.storage.data);
		}
	});
};

do we really need to pollute html tag with whole config setAttribute('it-' + key.replace(/_/g, '-'), items[key])? Menu has more elegant version of this setting few from a list

var attributes = {
theme: true,
improvedtube_home: true,
title_version: true,
it_general: true,
it_appearance: true,
it_themes: true,
it_player: true,
it_playlist: true,
it_channel: true,
it_shortcuts: true,
it_blocklist: true,
it_analyzer: true,
layer_animation_scale: false
};
for (var attribute in attributes) {
var value = satus.storage.get(attribute);
if (attribute === 'improvedtube_home') {
attribute = 'home-style';
}
if (satus.isset(value)) {
extension.skeleton.rendered.setAttribute(attribute.replace('it_', '').replace(/_/g, '-'), value);
}

@ImprovedTube
Copy link
Member

ImprovedTube commented Apr 11, 2024

I wrote this code :P

yes❤️. One day another reader might join us.

wishlist

missed the word "could"

I mean, sure
extension.storage.load = function
thanks! we can add another message/event then html-Attributes-Added. (As a comment, if nothing in ImprovedTube.init() web-accessible/init.js#L84-L113 doesnt depends on any CSS)

do we really need to pollute html tag with whole config

convenient/special, because it works before the DOM is loaded. Could start with the many appearance & cleaning features (compare UnHook for YouTube)

do we really need to pollute html tag with whole config

could of course pre-process well
(and #1685)

cant use chrome.storage.local.get in content script injected into YT, its only available in private side of extension
cant override window.MediaSource.isTypeSupported from extension/core.js its a wrong execution context
we would have to inject javascript tag here to override MediaSource

*Can use message for both, bi-directionally. So the feature runs async.
( if the speed is required for storage-loaded)

@raszpl
Copy link
Contributor Author

raszpl commented Apr 21, 2024

do we really need to pollute html tag with whole config

convenient/special, because it works before the DOM is loaded.

but does anything use it? :) So I decided to check. There is this:

Thats all for js code, rest are pure CSS uses:

  • theme
  • player-crop-chapter-titles
  • ads
  • mini-player-cursor
  • player-fit-to-win-button
  • channel-hide-featured-content
  • schedule

Thats all extension ever uses, so we can go:

		let htmlAttributes = [
			"pathname",
			"player-size",
			"theme",
			"player-crop-chapter-titles",
			"ads",
			"mini-player-cursor",
			"player-fit-to-win-button",
			"channel-hide-featured-content",
			"schedule"
		];

		for (const key in items) {
			if (htmlAttributes.includes(key)) {
				document.documentElement.setAttribute('it-' + key.replace(/_/g, '-'), items[key]);
			}
		}

*Can use message for both, bi-directionally. So the feature runs async.

thats how it was done previously, messages were too slow

@raszpl
Copy link
Contributor Author

raszpl commented Apr 21, 2024

correction, I forgot CSS is in multiple small files, there is more used by CSS. Whole list:
"activated",
"ads",
"always-show-progress-bar",
"bluelight",
"channel-compact-theme",
"channel-hide-featured-content",
"collapse-of-subscription-sections",
"columns",
"comments",
"comments-sidebar",
"comments-sidebar-left",
"comments-sidebar-simple",
"compactSpacing",
"description",
"embeddedHidePauseOverlay",
"embeddedHideShare",
"embeddedHideYoutubeLogo",
"header-hide-country-code",
"header-hide-right-buttons",
"header-improve-logo",
"header-position",
"header-transparent",
"hide-animated-thumbnails",
"hide-author-avatars",
"hide-clip-button",
"hide-comments-count",
"hide-date",
"hide-details",
"hide-dislike-button",
"hide-download-button",
"hide-footer",
"hide-gradient-bottom",
"hide-more-button",
"hide-playlist",
"hide-report-button",
"hide-save-button",
"hide-scroll-for-details",
"hide-share-button",
"hide-shorts-remixing",
"hide-sidebar",
"hide-thanks-button",
"hide-thumbnail-overlay",
"hide-video-title-fullScreen",
"hide-views-count",
"hide-voice-search-button",
"improvedtube-search",
"likes",
"livechat",
"mini-player-cursor",
"no-page-margin",
"pathnam",
"pathname",
"player-autoplay-button",
"player-color",
"player-crop-chapter-titles",
"player-fit-to-win-button",
"player-hide-annotations",
"player-hide-cards",
"player-hide-endscreen",
"player-hide-skip-overlay",
"player-miniplayer-button",
"player-next-button",
"player-play-button",
"player-previous-button",
"player-remote-button",
"player-screen-button",
"player-settings-button",
"player-show-cards-on-mouse-hover",
"player-size",
"player-size",
"player-subtitles-button",
"player-transparent-background",
"player-view-button",
"player-volume-button",
"red-dislike-button",
"related-videos",
"remove-black-bars",
"remove-history-shorts",
"remove-home-page-shorts",
"remove-related-search-results",
"remove-shorts-reel-search-results",
"remove-subscriptions-shorts",
"remove-trending-shorts",
"schedule",
"scroll-bar",
"scroll-to-top",
"search-focus",
"sidebar-left",
"squared-user-images",
"subscribe",
"theme",
"thumbnails-hide",
"thumbnails-right",
"transcript",
"youtube-home-page",
"youtubeDetailButtons"

@raszpl
Copy link
Contributor Author

raszpl commented Apr 21, 2024

"no-page-margin", "pathnam", "pathname", "player-autoplay-button"

well, would you look at that, fished out a bug!

html[it-pathnam='/feed/history'][it-remove-history-shorts="true"] ytd-reel-shelf-renderer span#title:contains("Shorts"),

fix: #2196
But pathname is not even a setting, its initialized in init.js

document.documentElement.setAttribute('it-pathname', location.pathname);

so it will never be matched here when loading settings, can remove it from my list :) Wonder if any other items on a list are also like that.

With #2197 and my extension config during chrome.storage.local.get the only Attribute injected to HTML tag is theme, much less pollution and less blocking.

@raszpl
Copy link
Contributor Author

raszpl commented Apr 21, 2024

ok, so I tried really hard to make injection from /extension/core work, but it looks like chrome.storage.local.get timing fluctuates, around 50% of the time it runs really fast, other times it returns good 800ms after DOMContentLoaded

		let MediaElementOverride = document.createElement('script');
		MediaElementOverride.type = "text/javascript";
		let codec = {block_vp9:items['block_vp9'], block_h264:items['block_h264'], block_av1:items['block_av1']},
			player_60fps = items['player_60fps'] ? true : false;
/* original:
	function overwrite(self, callback, mime) {
		let atlas = {block_vp9:'vp9|vp09', block_h264:'avc1', block_av1:'av01'},
			store = !ImprovedTube?.storage ? XXXX : ImprovedTube.storage,
			codec = Object.keys(atlas).reduce(function (all, key) {
				return store[key] ? ((all?all+'|':'') + atlas[key]) : all}, ''),
			player_60fps = !ImprovedTube?.storage ? YYYY : ImprovedTube.storage.player_60fps;

		if (codec) {
			var re = new RegExp(codec);
			// /webm|vp8|vp9|av01/
			if (re.test(mime)) return '';
		}
		if (player_60fps) {
			var match = /framerate=(\d+)/.exec(mime);
			if (match && match[1] > 30) return '';
		}
		return callback.call(self, mime);
	};

	if (window.MediaSource) {
		let isTypeSupported = window.MediaSource.isTypeSupported;
		window.MediaSource.isTypeSupported = function (mime) {
			return overwrite(this, isTypeSupported, mime);
		}
	}
	let canPlayType = HTMLMediaElement.prototype.canPlayType;
	HTMLMediaElement.prototype.canPlayType = function (mime) {
		return overwrite(this, canPlayType, mime);
	}
*/
		MediaElementOverride.innerHTML = "{console.log('injection'); function overwrite(self, callback, mime) {let atlas = {block_vp9:'vp9|vp09', block_h264:'avc1', block_av1:'av01'}, store = !ImprovedTube?.storage ? "+ JSON.stringify(codec) +" : ImprovedTube.storage, codec = Object.keys(atlas).reduce(function (all, key) { return store[key] ? ((all?all+'|':'') + atlas[key]) : all}, ''), player_60fps = !ImprovedTube?.storage ? "+ player_60fps +" : ImprovedTube.storage.player_60fps; if (codec) { var re = new RegExp(codec); if (re.test(mime)) return ''; } if (player_60fps) { var match = /framerate=(\d+)/.exec(mime); if (match && match[1] > 30) return ''; } return callback.call(self, mime); }; if (window.MediaSource) { let isTypeSupported = window.MediaSource.isTypeSupported; window.MediaSource.isTypeSupported = function (mime) { return overwrite(this, isTypeSupported, mime); } } let canPlayType = HTMLMediaElement.prototype.canPlayType; HTMLMediaElement.prototype.canPlayType = function (mime) {return overwrite(this, canPlayType, mime);} };";
		document.documentElement.appendChild(MediaElementOverride);

cant make it work reliably without localstorage :(

@ImprovedTube ImprovedTube merged commit 9c4e5f6 into code-charity:master Apr 21, 2024
@ImprovedTube
Copy link
Member

hi! @raszpl

thats how it was done previously, messages were too slow

wasn't it our storage-loaded that was slow?


fluctuates,

i think it rate limits us while testing "too quickly ( /also one trigger for #1803)


(
could be this alone first:

chrome.storage.local.get('HTMLMediaElementSettingsCodecFpsEtc', function (result) { ...

(if chrome.storage.local.get(null) or extension.storage.load = function (callback) { chrome.storage.local.get(function (items) { extension.storage.data = items; are any slower
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants