From 6d99a39190abb1b1ba2ea1df68990db773326bfb Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Sat, 22 Aug 2026 19:56:48 +0200 Subject: [PATCH] Do not list a moved event occurrence twice An occurrence of a recurring event that was changed on its own carries a RECURRENCE-ID and is stored as an additional VEVENT next to the event it belongs to. `getAllSubcomponents("vevent")` returns those components as well, and since they have no RRULE of their own they were handled as separate single events. The occurrence therefore ended up in the list twice: once from expanding the recurring event, which resolves the modified time through the exception, and once more from the extra VEVENT. To make it worse the two rows disagreed, because name and URL were taken from the recurring event while the time came from the modification, so the first row showed the new time under the old name. Skip components that are a recurrence exception, they are already covered by the event they modify, and take name and URL from the occurrence details, which point at the modification where there is one and at the event itself otherwise. Events whose RECURRENCE-ID refers to an event that is not in the file are dropped by this, which cannot happen in a full calendar export. With a recurring Plenum whose 25.08. occurrence is moved two hours earlier and renamed: before: 25.08. 18:00 CCCB Plenum 25.08. 18:00 CCCB Plenum (verschoben) after: 25.08. 18:00 CCCB Plenum (verschoben) Leaning on the resolution ical.js does here needs two more things to be right. The first is which modifications an event is asked to resolve. Unless it is told, `ICAL.Event` relates every VEVENT with a RECURRENCE-ID in the file to every recurring event and keys them by the recurrence id alone; the UID is only compared with `strictExceptions`, which then throws instead of skipping. A modification would therefore also override the occurrence another series holds at the same instant, so the wrong entry is shown and the modified one twice over. Group the modifications by the UID of the event they belong to and hand each event its own, which also turns the relating off for events that have none. Recognise a modification on the component instead of on the event, because relating exceptions to an exception throws. With two unrelated weekly series that both meet on Thursday at 19:00, of which only A has its occurrence of 03.09. moved to 17:00: before: 27.08. 19:00 Serie A after: 27.08. 19:00 Serie A 27.08. 19:00 Serie B 27.08. 19:00 Serie B 03.09. 17:00 Serie A (versch.) 03.09. 17:00 Serie A (versch.) 03.09. 17:00 Serie A (versch.) 03.09. 19:00 Serie B The second is how far the expansion has to run. It walks the unmodified recurrence times and stopped at the end of the window, but the occurrence handed to `getOccurrenceDetails()` may have been moved somewhere else entirely. Both directions were wrong: an occurrence pulled forward from beyond the window was never reached, because the walk had already stopped at its original time, and one pushed out of the window was still listed, because only its original time was ever compared against the window. Iterate far enough that the largest move towards the past can still reach the window, and decide by the time the occurrence really takes place at. The stop condition keeps using the raw recurrence time, which stays monotonic, so the iteration still terminates. With a weekly series whose occurrence of 05.10. is pulled forward to 25.08. and whose occurrence of 31.08. is pushed to 02.11., seen from 23.08. through the 20 day window: before: 24.08. Serie after: 24.08. Serie 31.08. Serie 25.08. Serie (vorgezogen) 07.09. Serie 07.09. Serie Reading the URL becomes a function of its own while name and URL move to the occurrence details, and it now looks at what it reads. The calendar is exported from a CalDAV server, so whoever may write to it decides what ends up in the href of the link, and `URL:javascript:alert(1)` on an event would run that script when a visitor clicks the entry. Pass on nothing but http and https; a value that is not a URL at all no longer reaches the href either. The published calendar contains no RECURRENCE-ID at all today, so nothing about the modifications changes for it. It is exported from a CalDAV server though, and moving a single Club Discordia or Plenum out of the way of a holiday is exactly what creates such a modification. Checked against https://berlin.ccc.de/calendars/all.ics: the list and the links of the 30 events that carry a URL are unchanged. Fixes: c28f04c6e85c ("switch to ics files; make calendars work; fix some minor issues") Assisted-by: Claude:claude-opus-5 Signed-off-by: Hauke Mehrtens --- static/js/upcoming.js | 125 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 114 insertions(+), 11 deletions(-) diff --git a/static/js/upcoming.js b/static/js/upcoming.js index f031b4d..dde5f1f 100644 --- a/static/js/upcoming.js +++ b/static/js/upcoming.js @@ -1,5 +1,96 @@ import ICAL from "https://unpkg.com/ical.js/dist/ical.min.js"; +/** + * Read the URL of an event. + * + * ICAL.Event does not expose the URL property, so read it from the component. + * + * The value ends up in the href of a link, and the calendar is exported from a + * CalDAV server, so whoever may write to it decides what that value is. A + * "javascript:" URL there would run on our page as soon as a visitor clicks + * the event, so hand on nothing but http and https. + * + * @param {ICAL.Event} event The event to read the URL of + * @returns {string} The URL, empty when the event has none or it is not http(s) + */ +function eventUrl(event) { + const url = event.component.getFirstPropertyValue("url") ?? ""; + + if (!url) { + return ""; + } + + try { + // A relative URL is resolved against the page and keeps its scheme. + const { protocol } = new URL(url, document.baseURI); + + return protocol === "http:" || protocol === "https:" ? url : ""; + } catch { + // Not a URL at all. + return ""; + } +} + +/** + * Group the occurrences that were modified on their own by the UID of the event + * they belong to. + * + * Unless it is told which exceptions belong to an event, ICAL.Event relates + * every VEVENT with a RECURRENCE-ID in the file to every recurring event, and + * it keys them by the recurrence id alone. Two series that meet at the same + * time would therefore take over each other's modifications. + * + * @param {ICAL.Component[]} components The VEVENTs of the calendar + * @returns {Map} The exceptions per UID + */ +function exceptionsByUid(components) { + const exceptions = new Map(); + + for (const component of components) { + if (!component.hasProperty("recurrence-id")) { + continue; + } + + const uid = component.getFirstPropertyValue("uid"); + const ofEvent = exceptions.get(uid); + + if (ofEvent) { + ofEvent.push(component); + } else { + exceptions.set(uid, [component]); + } + } + + return exceptions; +} + +/** + * How far the recurrences of an event have to be iterated. + * + * The iteration walks the unmodified recurrence times, so an occurrence that + * was moved to an earlier time is only reached through the time it originally + * had, which can lie past the end of the window. Keep going for as long as the + * largest move towards the past can still carry an occurrence into it. + * + * @param {ICAL.Event} event The event whose recurrences are iterated + * @param {Date} end End of the window + * @returns {Date} The recurrence time to stop at + */ +function iterationEnd(event, end) { + let last = end.getTime(); + + for (const exception of Object.values(event.exceptions)) { + const movedBy = exception.recurrenceId.toJSDate().getTime() + - exception.startDate.toJSDate().getTime(); + + if (movedBy > 0) { + last = Math.max(last, end.getTime() + movedBy); + } + } + + return new Date(last); +} + /** * Parse an ICS calendar and return upcoming event occurrences. * @@ -18,18 +109,27 @@ function getUpcomingEvents(icsText, now, maxEvents, maxDays) { const events = []; - for (const component of calendar.getAllSubcomponents("vevent")) { - const event = new ICAL.Event(component); + const components = calendar.getAllSubcomponents("vevent"); + const exceptions = exceptionsByUid(components); + + for (const component of components) { + // Occurrences modified via RECURRENCE-ID are reached through the event they + // belong to, listing them here as well would show them twice. + if (component.hasProperty("recurrence-id")) { + continue; + } + + const event = new ICAL.Event(component, { + exceptions: exceptions.get(component.getFirstPropertyValue("uid")) ?? [], + }); if (!event.startDate) { continue; } - // ICAL.Event does not expose the URL property, so read it from the component. - const url = component.getFirstPropertyValue("url") ?? ""; - if (event.isRecurring()) { const iterator = event.iterator(); + const iterateUntil = iterationEnd(event, end); while (true) { const occurrence = iterator.next(); @@ -40,19 +140,22 @@ function getUpcomingEvents(icsText, now, maxEvents, maxDays) { // Recurrences are chronological, so we're done // once we pass the end of our search window. - if (occurrence.toJSDate() > end) { + if (occurrence.toJSDate() > iterateUntil) { break; } - // Details resolve the times of occurrences overridden by RECURRENCE-ID. + // Details resolve time, name and URL of an occurrence that was + // modified via RECURRENCE-ID. const details = event.getOccurrenceDetails(occurrence); // A running event stays listed until it is over, so filter on its end. - if (details.endDate.toJSDate() > now) { + // A modification may have moved the occurrence out of the window, so + // judge it by the time it really takes place at. + if (details.startDate.toJSDate() <= end && details.endDate.toJSDate() > now) { events.push({ start: details.startDate.toJSDate(), - name: event.summary ?? "", - url, + name: details.item.summary ?? "", + url: eventUrl(details.item), }); } } @@ -63,7 +166,7 @@ function getUpcomingEvents(icsText, now, maxEvents, maxDays) { events.push({ start, name: event.summary ?? "", - url, + url: eventUrl(event), }); } }