AllMangasReader-dev / AMR

AllMangasReader developer branch
www.allmangasreader.com
Other
99 stars 56 forks source link

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

Open KriPet opened 11 years ago

KriPet commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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

braiam commented 11 years ago

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 notifications@github.com

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/AllMangasReader-dev/AMR/issues/76#issuecomment-17083077 .

Braiam Peguero

KriPet commented 11 years ago

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 commented 11 years ago

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.

braiam commented 11 years ago

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

fuzetsu commented 11 years ago

Sure, I'll take a look!

braiam commented 11 years ago

Assigning @fuzetsu , dunno who assigned me

fuzetsu commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 9 years ago

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