Add-on calendars not added to composite after restart
Categories
(Calendar :: Internal Components, defect)
Tracking
(thunderbird_esr78 fixed, thunderbird86 wontfix, thunderbird87 fixed)
People
(Reporter: public, Assigned: public)
References
Details
Attachments
(4 files, 2 obsolete files)
1.50 KB,
text/x-python
|
Details | |
1.25 KB,
text/x-python
|
Details | |
3.82 KB,
patch
|
darktrojan
:
review+
|
Details | Diff | Splinter Review |
3.82 KB,
patch
|
public
:
review+
wsmwk
:
approval-comm-esr78+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1673280 +++
A tainted startup cache can prevent events in add-on-calendars from appearing in the composite calendar, although the calendar remains visible. This is different from bug 1673280, which can cause calendars to become 'properly' invisible on restarts (crossed eye in the UI).
STR:
- Clean Profile, Thunderbird 78.5.0
- Create a new contact with a display name and a birthday in the near future
- Install the Birthday Calendar Add-on 1.0 from https://github.com/rsjtdrjgfuzkfg/thunderbird-birthdaycalendar/releases/download/v1.0/birthdaycalendar.xpi
- Observe that two calendars were created (for the default address books), and verify that you can see the birthday
- Quit Thunderbird by closing its window.
- Delete the startup cache file associated with the Thunderbird profile you're testing with:
~/.cache/Thunderbird/*/startupCache/startupCache.8.little
on Linux,%LOCALAPPDATA%\Thunderbird\Profiles\*\startupCache\startupCache.8.little
on Windows - Start Thunderbird (you should not see changes, the event is still there).
- Quit Thunderbird by closing its window.
- Start Thunderbird again.
Expected:
The calendar remains marked as visible (no crossed eye icon) and the birthday is visible.
Actual:
The calendar remains marked as visible but the birthday is not visible. Hiding the calendar and making it visible again fixes the issue for one session, but the birthday will again become hidden on restarts.
Workaround:
Invalidating the startup cache fixes the issue: open the error console (Ctrl+Shift+J) and evaluate the statement Services.obs.notifyObservers(null, "startupcache-invalidate", null);
, then restart Thunderbird.
Assignee | ||
Comment 1•4 years ago
|
||
I did quite a bit of debugging on this. I'm still not sure how the startup cache works in detail, but I managed to understand the file format sufficiently to build packing / unpacking tools and compare cache contents.
This bug seems related to a combination of startup cache entries starting with xulcache/resource/gre/chrome/calendar/content/
Removing these entries fixes the error in a clean test profile (but the entries are not regenerated, so there could be a performance penalty). Partially removing entries in that area has mixed results. The workaround (startup cache invalidation) removes much more entries, but has otherwise the same effect (most entries are not re-generated!).
Now here's the weird part: the entries in that 'folder' are binary-identical with the cache from my productive installation of Thunderbird, which is not affected by this bug. There are a few binary differences in other cache entries (from a hex editor they seem to have a different order of class members, no idea why), though, and quite a few additional cache entries on each side. Copying every cache entry from my productive profile does not seem to fix the test profile – maybe due to profile specific data in caches.
During these experiments, I was also able to verify that add-on caching is not the issue: selectively clearing cache entries of the add-on does not influence the bug. Furthermore, I have experienced many inconsistent situations where the bug was triggered with a cache state that did not previously trigger it, or vice versa. That could just be a side-effect of directly messing with internal caches, or point in the vague direction of a caching-related timing problem.
I'd appreciate if somebody with more toolkit experience could have a look at this :)
Assignee | ||
Comment 2•4 years ago
|
||
This a rather dirty script to unpack the startup cache files to a folder (requires lz4 command line tools and a somewhat recent python), if somebody wants to have a shot at this.
Assignee | ||
Comment 3•4 years ago
|
||
This a rather dirty script to re-pack a folder into a startup cache file (requires lz4 command line tools and a somewhat recent python), if somebody wants to have a shot at this.
Assignee | ||
Comment 5•4 years ago
•
|
||
The issue seems to be that composite calendars are created via cal.view.getCompositeCalendar(window)
, without any mechanism to keep their calendar lists updated. That task is currently handled by calendar-management.js. So composite calendars can get out of sync if calendars are registered / unregistered before something calls loadCalendarManager()
in their associated window.
I have no idea why we have separate instances of the main composite calendar for each window, but assuming that was a intentional decision the best fix I came up with is registering an observer for each composite calendar instance. The attached fix is for 78, I'll attach a rebased version for comm-central in the next minutes.
At least in my tests, I can no longer reproduce the issue once the patch is applied.
Assignee | ||
Comment 6•4 years ago
|
||
Rebased fix for comm-central.
Comment 7•4 years ago
|
||
The composite calendar is per-window because it is used to control the list of selected calendars. The idea was that you might have a different set of selected calendars per window.
Assignee | ||
Comment 8•4 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] [:📆][:🧩] from comment #7)
The composite calendar is per-window because it is used to control the list of selected calendars. The idea was that you might have a different set of selected calendars per window.
But they all store the visibility information in the same pref (as it is the main composite calendar), or is that a bug? From the interface design I'd have assumed that any scope that needs a different set of visible calendars either uses anonymous composite calendars, or composite calendars with names ("prefPrefix") other than "calendar-main". Imho it would make more sense to have the calendar manager own all named composite calendars (and both keep them updated and enforce a single instance for each prefPrefix).
Either way, it would be good to get a fix merged and (hopefully) uplifted soon, as this bug affects many non-technical users that struggle to apply the cache clearing workaround.
Comment 9•4 years ago
|
||
Comment on attachment 9203692 [details] [diff] [review]
fix against comm-central
Nice find and fix.
Comment 10•4 years ago
|
||
Comment on attachment 9203691 [details] [diff] [review]
fix against comm-esr78
This is identical to the other patch apart from a first line of context, and doesn't really need review.
Assignee | ||
Comment 11•4 years ago
|
||
Fixed patch to make the linter happy. Sorry about that!
Comment 12•4 years ago
|
||
Comment on attachment 9204102 [details] [diff] [review]
fix against comm-central, v2
You can carry forward reviews for simple changes like this one.
Assignee | ||
Comment 13•4 years ago
|
||
Update for the patch against comm-esr with the same linter change
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 14•4 years ago
|
||
Comment on attachment 9204105 [details] [diff] [review]
fix against comm-esr78, v2
[Approval Request Comment]
User impact if declined: Users with add-on calendars might feel like they lost all events; users that already know the bug need to continue to re-apply the workaround after each update of Thunderbird
Testing completed (on c-c, etc.): tested local builds of comm-central and com-esr78 with the patch applied; bug is gone and the observer seems to get cleanly registered/unregistered. I found the related bug 1693713, though (present independently of applying the patch)...
Risk to taking this patch (and alternatives if risky): seems not really risky to me, as no core logic was changed
Comment 15•4 years ago
|
||
I'm about to land this. Just waiting for m-c.
Comment 16•4 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/9d7dc409eccd
Add observer for each main composite calendar, r=darktrojan
Comment 17•3 years ago
|
||
Comment on attachment 9204105 [details] [diff] [review]
fix against comm-esr78, v2
[Triage Comment]
Approved for esr78
Comment 18•3 years ago
|
||
bugherder uplift |
Thunderbird 78.8.1:
https://hg.mozilla.org/releases/comm-esr78/rev/2aaa02f5ac8a
Description
•