Fix for some blank accounts, change activeUserId pref to char to fix integer overflow

[YoruNoHikage]

This fixes the #51, can you test it ?

[modem]

Hi,

thanks for the fix, but how to test it?

[modem]

Did the following: Downloaded the branch with the fix, extracted it and re-zipped it to strip the base folder, and renamed to .xpi in about:config, changed the flag xpinstall.signatures.required to false to allow the installation of unsigned addons Installed the new .XPI file, but Firefox crashed during installation, and it was crashing all the times i've started it until I deleted the folder echofon-unofficial@echofox-team in the profile extensions folder.

[YoruNoHikage]

There's a build script you can use, but if you're on windows, you have to do everything by hand. We should rewrite this one though... Anyway, did you do all these steps ?

• zip chrome/Echofon/* and rename to chrome/Echofon.jar
• zip everything and rename to <anything>.xpi
• install and relaunch Firefox

Should work with that!

[modem]

Thank you @YoruNoHikage, I was not aware of the 1st step you mentioned. So, I just did like you said and I was able to install it, but after restarting Firefox, it keeps telling me that I need to restart to load the new addon (see picture attached)

[modem]

Ok, I just fixed it. Closed Firefox, edited the prefs.js from my profile folder to change user_pref("extensions.twitternotifier.activeUserId", MY_ID_HERE); to user_pref("extensions.twitternotifier.activeUserId", "MY_ID_HERE"); and after launching Firefox again it was working. I can now access to my new account in Echofox.

Thank you for fixing it.

[YoruNoHikage]

Was this your code or every users will be forced to do such an action ? We should have transparent updates. :confused:

[modem]

These were the settings already present in Firefox, with the activeUserId defines as an integer. I forced it to be a string, so probably all users will need to do this if they upgrade an existing addon installation. I think you need to force this change, at least during the upgrade (if possible).

[YoruNoHikage]

Maybe we should use a new preference instead, it seems you can't force update a pref...

[ath0mas]

Yes, existing pref has type Int and calling setCharPref on it throws exception :( With a new pref name it should be fine. We should migrate existing value from old pref to new pref?

See "migrateV1toV2" function in nsEchofon.js: it may be possible to read Int value, insert toString() into new pref, and finally deleteBranch? or, to keep same pref name: read, deleteBranch and finally insert toString() into new pref with same name?

[YoruNoHikage]

Oh didn't see that ! I'll try something. :)

[rrglomsk]

Ooh I'm glad this issue is being looked into, I've had this problem with my extra accounts for a year or so now and sort of forgot about it heh. Looking forward to a solution :smile:

[YoruNoHikage]

This issue is fixed, we just have to write a migration script and that's all! ;)

[ath0mas]

I just added a commit to this pull request, to Migrate int activeUserId to char activeUserIdStr I messed up my persisted account in Echofon, in my Firefox for Dev, so I can not test it fully :( Anybody to validate please? And changes I mentioned @YoruNoHikage?

[YoruNoHikage]

Sounds good to me, the new pref is here, the old one is gone. And for the changes, I think you're right, we may need to change to check empty string, is 0 set by Echofon when nothing is selected? I forgot this part...

[ath0mas]

if (EchofonCommon.pref().getCharPref("activeUserIdStr") == 0) { replaced, but missing !?

[ath0mas]

And should we replace checks on BoolPrefs too?

[YoruNoHikage]

I was too focused on removing == that I completely forgot to put !.

Can you correct it directly on master? Also, we can remove it for BoolPref but it's just look nicer haha.

[ath0mas]

Sure ;)

[ath0mas]

I would like to step back on replacing EchofonCommon.pref().getCharPref("activeUserIdStr") == '' as we do not always use empty string as the default value for CharPrefs, like "{}" for getCharPref("accounts")

[ath0mas]

Diving in our changes about activeUserId, I found that we should add changes linked to user_id globally?

in window.js,function activeUser() now returns a string and not an int - everywhere used should be verified :disappointed: like,

window.js:

• if (user_id == 0) line 132, check '' instead

Models.jsm:

• EchofonModel.User.findById -> bindInt64Parameter does not deal with int anymore
• type of id, recipient_id, db_uid at least sometimes (or always?) string now

[YoruNoHikage]

Hum... As you wish, for this pref, it's only empty string or string of numbers, so...

Argh, I'll take a look today...

[ath0mas]

To better understand the situation here is the resource on Twitter side about user_id change we have to deal with: https://dev.twitter.com/overview/api/twitter-ids-json-and-snowflake https://blog.twitter.com/2013/moving-to-64-bit-twitter-user-ids

If you develop in Javascript, you should read the string version of Twitter IDs, instead of the integer version.

not easy, but would be great to master when we read/use id or id_str, and then our user_id as an int32 (to forbid), int64, or string

[YoruNoHikage]

There are already some things that have been made for this. https://github.com/echofox-team/echofon-firefox-unofficial/blob/752c066cf9a30de682a3bb6763fa643e3c2a2aa1/modules/Models.jsm#L19-L37