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

Syncing using bookmark don't allow more than 7.5k characters #76

Open
KriPet opened this issue Apr 26, 2013 · 22 comments
Open

Syncing using bookmark don't allow more than 7.5k characters #76

KriPet opened this issue Apr 26, 2013 · 22 comments
Assignees
Labels

Comments

@KriPet
Copy link

KriPet commented Apr 26, 2013

It seems chrome will not sync the long bookmarks created by bsync. I managed to track down the max length of a bookmark to between 7645 and 7887 characters.

I made a lot of bookmarks with varying amount of mangas in them. The bookmark with 7645 characters managed to sync, the one with 7887 did not.

I also confirmed that encoding is NOT an issue, as all my mangas synced fine when divided into bookmarks smaller than 7887 characters.

chrome version: 28.0.1485.0 dev-m

@braiam
Copy link
Member

braiam commented Apr 26, 2013

Ok, the size of the bookmark seems to be the problem (no more of 7.5kbytes?). so the only solution is splitting the bookmark or compressing it.

@bessx
Copy link
Member

bessx commented Apr 26, 2013

why not js split the bsync file into a few diff bookmarks? every time it gets over 7kb? at least till we get the other thing working

@KriPet
Copy link
Author

KriPet commented Apr 26, 2013

Yeah, that seems like the way to go. I can look into it, but as I have said before, JS is not my best programming language.

I'll see what I can do though.

One question. Which would be the best way to make the bookmark?

  1. Try creating a single bookmark, check size, if > 7kB: split into size/7000 bookmarks
  2. Guess at the largest size an entry would have and make bookmarks with 7000/guessed_size entries until there are no more entries

I think the first one would be slow, but the second one would fail if a user uses a lot of categories or has a list full of long named mangas with long last chapter links.

Any preferences?

@bessx
Copy link
Member

bessx commented Apr 26, 2013

Probably better to js do a count on the intended items before creating the bookmark.

@braiam
Copy link
Member

braiam commented Apr 26, 2013

Remember that isn't items but total size of the string.

Probably the best solution is take the string and split it if is more than
7k characters long, then create 2 bookmarks, the first part of the string
and the rest, then when receiving take all bookmarks if they are splitted
and join all the strings and process like usual. But we would have to check
when there are 2 bookmarks if the first one is less than the limit so we
have to ignore the second one.

Or just using 2 bookmarks: for the mangas and for the bookmarks separated.

2013/4/26 Someone516 [email protected]

Probably better to js do a count on the intended items before creating the
bookmark.


Reply to this email directly or view it on GitHubhttps://github.com//issues/76#issuecomment-17083077
.

Braiam Peguero

@KriPet
Copy link
Author

KriPet commented Apr 26, 2013

Even when splitting mangas and bookmarks, my mangas string would be too long; but it could still be a nice idea, splitting the mangas into one set of bookmarks and the manga-bookmarks into another.

I won't have time to look at this today. Maybe tomorrow.

@KriPet
Copy link
Author

KriPet commented Apr 27, 2013

OK, looking at the code for BSync, I realize I am way over my head here. I'm sorry, but it seems I won't be as much help as I'd like.

@ghost ghost assigned braiam Apr 28, 2013
@braiam
Copy link
Member

braiam commented Apr 28, 2013

@fuzetsu could you check if your decode/encode function can be applied here? Just to have the thing working again.

@fuzetsu
Copy link
Contributor

fuzetsu commented Apr 29, 2013

Sure, I'll take a look!

@ghost ghost assigned fuzetsu Apr 29, 2013
@braiam
Copy link
Member

braiam commented Apr 29, 2013

Assigning @fuzetsu , dunno who assigned me

@fuzetsu
Copy link
Contributor

fuzetsu commented Apr 30, 2013

I've taken the last half hour to read through the code for Bsync.js and background.js

I can see why KriPet (or anyone) would feel overwhelmed by it. More than 1000 lines of confusing/uncommented code with unhelpfully named variables (_55, _56, etc) ...

I've more or less grasped what it does and how it does it and I'm feeling that it's going to be a pain to alter to fit our purposes. To begin with Bsync is someone else's code imported into AMR supposedly as a generic solution for syncing data in chrome extensions. I'll continue working my way through to see if there's an easy way to work what we want in it but it's possible that a rewrite and simplification is in order (which I guess could also mean just starting work on storage.sync).

@fuzetsu
Copy link
Contributor

fuzetsu commented Apr 30, 2013

Actually I just found the site where BSync is from and it looks like it was updated. I'll see if the update changes anything regarding this issue. http://phaistonian.pblogs.gr/2010/06/bsync-syncing-for-chrome-extensions-part-ii.html

At least this version has comments :-P

@KriPet
Copy link
Author

KriPet commented Apr 30, 2013

In addition to being uncommented and using weird variable names, it struck me as weird that the code for mergeing the new list with the old one is not in BSync.js, but in background.js. I know this is a minor thing, but if you are looking into fixing this, and agree with me that everything sync-related should be in one place, maybe you can move it?

@fuzetsu
Copy link
Contributor

fuzetsu commented Apr 30, 2013

If/when we make our implementation we would definitely move the logic over to one spot but from what I understand the point of Bsync is that you simply drop it as is into your project as is and then in your own script create an instance of it and define the relevant functions to match your use case. Supposedly, all the code in Bsync is generic and should work for any implementation. Taking that into account I find it curious that so far I haven't found anything that handles the "bookmark too large to sync" issue. Then again this is just a small library that this guy made on his free time (at least it seems that way).

@fuzetsu
Copy link
Contributor

fuzetsu commented Apr 30, 2013

Just found a part finally mentioning the issue, according to the script "content size can not exceed 2.2k".

Maybe all that it takes is just calling this.write multiple times in background.js?

EDIT: scratch that, the implementation binds itself to one bookmark...

@KriPet
Copy link
Author

KriPet commented Apr 30, 2013

About the implementation in one place:
I was wrong, it just seemed to me that the BSync.js file was already edited for AMR, with the ShouldRead functions and the like, but I see now that the only thing changed is probably the name of the bookmark.

About calling write multiple times:
Don't think that would work, as the script creates the bookmark correctly, but chrome won't sync it
I think you need to use multiple bookmarks. (Or compress, as mentioned before)

@fuzetsu
Copy link
Contributor

fuzetsu commented Apr 30, 2013

Agreed, I was thinking that calling write twice might be a step in the right direction though since that's the method that creates the bookmark. I think it would successfully create 2 bookmarks but by reading the code for "traverse" I noticed that it deletes all other bookmarks in the folder apart from the one it's bound to (which i think is decided by age).

@KriPet
Copy link
Author

KriPet commented Apr 30, 2013

A way I might have fix this if I knew what to do, would be to add a parameter to the write function that specifies the bookmark name, and running it with names like "AMR1", "AMR2", but then you'd have to write a new read function as well.

@fuzetsu
Copy link
Contributor

fuzetsu commented Apr 30, 2013

Yeah, I was thinking something similar, we could do something like that or change Bsync to make and keep track of multiple bookmarks based on their size...

@arran4
Copy link

arran4 commented Oct 26, 2013

Last w/e in my fork I went over bsync.. I strongly advise not touching it a lot of it is very sensitive. I have been working on a GsSync.. Which is basically "Google Spreadsheet" sync.. I think it's almost ready to try out. It does have a limitation with deleting manga however. I need to speak to someone who knows the rest of the code base better than me. Any guinea pigs?

@codewisp
Copy link

Is there a way forward on this one? My bookmark sync isn't working at all...I basically export my data, put it on Dropbox, and import it on other computers if I actually want it to sync. Could maybe try to contribute something if I can get a grasp of what's going on.

@stardisblue
Copy link

we could simplify it by using the import export JSONS (each manga has it own timestamp) ? no ? 'cause the Bsync is real mess... or maybe add a button forcing the bookmark to be created

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

No branches or pull requests

7 participants