crittermike / shortkeys

A browser extension for custom keyboard shortcuts
https://www.shortkeys.app/
Other
1.02k stars 161 forks source link

Settings cannot save if they exceed the 8KB quota for storage.sync #67

Closed ambrt closed 6 years ago

ambrt commented 6 years ago

If you apply function to key that will have a lot of code, the extension will break.

The problem is that extension tries to write to sync, when sync has limit of size for each item (if i remember well quota is 8k).

crittermike commented 6 years ago

Can I see an example of one that's breaking for you? Just paste the code here (if it's not sensitive/private).

crittermike commented 6 years ago

https://stackoverflow.com/a/29732624/183929 has some information. Apparently the max length for individual items is 8k if you use storage.sync.

We could possibly switch to storage.local and rely on #55 (import/export) instead of sync. Or maybe still default to sync but fall back to local if it's too large.

AlexByte commented 6 years ago

storage.sync should be used. Synchronization of settings is convenient, it should not be discarded. If the limit is exceeded, you can create an additional item.

crittermike commented 6 years ago

@AlexByte but what about situations where a single shortcut exceeds the limit, such as if there is a JavaScript snippet that is very large. Are you suggesting that we should split that snippet in half, or would you be okay falling back to local storage in that case?

AlexByte commented 6 years ago

You can assign an bookmarklet instead of code.

AlexByte commented 6 years ago

@mikecrittenden This issue is about exceeding the limit with a single shortcut. Should ask @ambrt whether he needed to synchronize such snippets. Or can it, in general, come up with a solution with bookmarklet.

crittermike commented 6 years ago

Yeah, I do think that it probably makes a lot of sense to make each shortcut its own storage item instead of sticking them all in the same storage item, but it is still possible for a single shortcut to exceed the limit if the shortcut is to run custom code and the custom code is really long. You're right, they could put that in a bookmarklet, but that's not exactly a great workaround. I think at the very least we should detect that sync fails and fall back to local storage if and when that happens. Thoughts?

AlexByte commented 6 years ago

I'm against full switching to local.storage. There you can save only large snippets (>8K ) in order not to exceed the limit of 102,400 bytes eventually. And probably should show in UI that that they will not be synchronized. I do not think keeping each shortcut in a separate element is good, since there is a limit of 512 MAX_ITEMS.

crittermike commented 6 years ago

Ok awesome, so:

1) switch to making each shortcut a separate storage item 2) still default to sync 3) detect when a shortcut is too big to sync and fall back to local in that case, and alert the user that that shortcut is too large to sync

Yeah?

AlexByte commented 6 years ago

In my opinion, it's best not to synchronize shortcuts that lead to exceeding the limit of 8,192 QUOTA_BYTES_PER_ITEM and recommend using a bookmarklet. It's easier to implement. I use bookmarklets because you can not use extensions on your smartphone.

crittermike commented 6 years ago

Cool, I'm with you. Thanks for the feedback! I'm on it!

AlexByte commented 6 years ago

@mikecrittenden Maybe the author wants another. Also there is an option to save such shortcuts in local storage and again recommend using the bookmarklet.

crittermike commented 6 years ago

The author? You mean me?

AlexByte commented 6 years ago

The author of the issue.

AlexByte commented 6 years ago

Now json of my settings weighs 7,662 bytes, so I would not reject the option of splitting json into several items.

crittermike commented 6 years ago

Haha nice, you're right on the border. @ambrt feel free to weigh in!

AlexByte commented 6 years ago

https://github.com/mikecrittenden/chrome-shortkeys/issues/67#issuecomment-345473046 But in this case, you need to carefully consider supporting the loading of the old storage scheme. You can add an additional item and store the names of additional elements or add a similar field to the original json element.

AlexByte commented 6 years ago

Still, you can not save huge shortcutsbecause otherwise the limit of 102,400 QUOTA_BYTES will suffice only for 12 shortcuts the size of 8,192, and in fact there are other extensions that use sync.storage. You can study the experience of other extensions and i need to see how much I have left from the limit of 102,400 QUOTA_BYTES.

Now I understand why not all extensions support synchronization.

AlexByte commented 6 years ago

@mikecrittenden what's the size of your json with settings?

crittermike commented 6 years ago

Oh mine is tiny, like 1KB. I only use a few basic ones.

AlexByte commented 6 years ago

@mikecrittenden I found a library to store objects larger than is allowed by default. Also it uses LZ compression, which compresses 2.7 times in my case. LZW compresses 3.06 times. LZMA compresses 4.5 times. Base64 compresses 2 times.

crittermike commented 6 years ago

Wow this looks perfect!

On Nov 18, 2017 17:56, "AlexByte" notifications@github.com wrote:

I found a library to store objects larger than is allowed by default. Useful for Google Chrome extensions and apps. Also it uses LZW compression, which compresses 3 times https://jsfiddle.net/ryanoc/fpMM6/. https://www.npmjs.com/package/chrome-storage-largesync

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mikecrittenden/chrome-shortkeys/issues/67#issuecomment-345477598, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB-BfirehI7rmMeIbLY6_IZmimlh8Q-ks5s32C4gaJpZM4QiU0k .

AlexByte commented 6 years ago

Other my extensions use 339 + 1560 = 1899 bytes. What looks perfect? UPD: I was wrong, I thought that there was one storage for all extensions, but in fact, for each extension, it had its own storage.

crittermike commented 6 years ago

That library looks perfect I mean.

crittermike commented 6 years ago

As a quick fix since lots of people have emailed me about this, I merged a fix which successfully falls back to local storage when syncing fails, and alerts the user that everything saved but that it wouldn't sync.

I did NOT split the data up into multiple items, one per shortcut. That's a bigger lift and I'll have to think about the best way to model that data.

crittermike commented 6 years ago

Also documented here: https://github.com/mikecrittenden/chrome-shortkeys/wiki/FAQs-and-Troubleshooting#why-arent-my-settings-being-synced-to-my-other-chrome-installations

ambrt commented 6 years ago

@AlexByte Thank you for discussion and @mikecrittenden thanks for fix.

The use case for large code is one shortcut for many pages that has similar function.

Like to go to next page on many different sites i do many times :

try {
  //  site specific query
}  catch(err)
{
   console.log(err)
}
try {
    // another site specific query
} catch(err)
{
  console.log(err)
}

etc...

This is where i hit the limit of 8k per item.

I might use separate key-shortcut for each site but problem would be that it would take more time and effort then just adding another try/catch.
And with one long item user can just copy one block and save it to file for backup since the export function isn't there yet. So thats why i posted this issue.

AlexByte commented 6 years ago

@ambrt In the extension, only the first shortcut with the same combination is executed

crittermike commented 6 years ago

@AlexByte FWIW that was actually fixed in #73 and went out yesterday.

AlexByte commented 6 years ago

@mikecrittenden good

AlexByte commented 6 years ago

@ambrt You can backup using Storage Area Explorer.

ambrt commented 6 years ago

@AlexByte I did that. Thanks.

AlexByte commented 1 year ago

@crittermike Why don't you use library chrome-storage-largeSync, which is used, for example, by the extension User JavaScript and CSS? "largeSync exposes the same api schema as chrome.storage"

Wow this looks perfect! On Nov 18, 2017 17:56, "AlexByte" @.***> wrote: I found a library to store objects larger than is allowed by default. Useful for Google Chrome extensions and apps.

[Methods](https://github.com/dtuit/chrome-storage-largeSync#methods)
//get: Gets one or more items from storage.
largeSync.get(string or array of string or object keys, function callback);

//getBytesInUse: Gets the amount of space (in bytes) being used by one or more items.
largeSync.getBytesInUse(string or array of string keys, function callback);

//set : Sets multiple items.
largeSync.set(object items, function callback);

//remove: Removes one or more items from storage.
largeSync.remove(string or array of string keys, function callback);

//clear: Removes all items from storage.
largeSync.clear(function callback);

In general, settings synchronization is not used in shortkeys at all now?