979262 - Store large integer values as strings (you cannot set the XXX pref to the number YYY, as number pref values must be in the signed 32-bit integer range -(2^31-1) to 2^31-1)

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect, P1)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

With the latest sources, I saw the following error message when running mozmill tests:

TEST-START | c:\Mozilla\Coding\Code\comm-central\objdir-thunderbird-central\mozilla\_tests\mozmill\stage\testLocalICS.js | testLocalICS
...
setFieldValue: aElement.removeAttribute couldn't remove disabled from null e: TypeError: aElement is null
...
Error: [calICSCalendar] Backup failed, no calendarmanager:[Exception... "you cannot set the calendar.registry.63cd6ba6-89a2-4d79-bfdd-f703b0b9549a.uniquenum pref to the number 1393937451510, as number pref values must be in the signed 32-bit integer range -(2^31-1) to 2^31-1.  To store numbers outside that range, store them as strings.'you cannot set the calendar.registry.63cd6ba6-89a2-4d79-bfdd-f703b0b9549a.uniquenum pref to the number 1393937451510, as number pref values must be in the signed 32-bit integer range -(2^31-1) to 2^31-1.  To store numbers outside that range, store them as strings.' when calling method: [calICalendarManager::setCalendarPref_]"  nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)"  location: "JS frame :: resource://calendar/modules/calProviderUtils.jsm :: cPB_setProperty :: line 796"  data: no]

Seems like the switch to Preferences.jsm (bug 923026) uncovered this type of issue.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

As noted on IRC, this problem seems to have been there all along, but made visible with Preferences.jsm. We have a few other calendar properties that use date values.

Possible solutions:

* A wrapper around Preferences.jsm that saves larger integers as a tagged string pref and (de)serializes correctly (con: yet another wrapper function)
* The same, but only in the setProperty part of the calendar manager that saves the prefs (con: might be more places this could fail)


Archaeopteryx, do you want to create the patch for this?

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v1

This should take care. I've decided not to migrate the preferences from the ics provider, since they will be recreated if unset anwyay.

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Comment on attachment 8389369 [details] [diff] [review]
Fix - v1

Review of attachment 8389369 [details] [diff] [review]:
-----------------------------------------------------------------

::: calendar/base/src/calCalendarManager.js
@@ +752,5 @@
>          cal.ASSERT(name && name.length > 0, "Pref Name must be non-empty!");
>  
>          let branch = (getPrefBranchFor(calendar.id) + name);
> +
> +        if (typeof value == "number" && (value > MAX_INT || value < MIN_INT || value % 1 != 0)) {

You could use for the latter part: !Number.isInteger(value)
I know it's longer, but boolean.

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v2

Tree is closed, here is the patch for checkin.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/92a0bdf9c37f