**Please make sure that you have followed these 3 rules before
submitting your Pull Request:**
> 1. Base your pull requests against the `develop` branch.
Done.
> 2. Include these infos in the description:
>
> - Does the pull request solve a **related** issue?
No
> - If so, can you reference the issue like this `Fixes
#<issue_number>`?
> - What does the pull request accomplish? Use a list if needed.
> - If it includes major visual changes please add screenshots.
>
Render a strict allowlist of basic formatting tags (b, strong, i, em, u)
in news titles and descriptions, while neutralizing all other HTML.
Feeds such as The Atlantic encode emphasis as entities (<em>),
which html-to-text decoded to a literal <em> string that the template
then auto-escaped, so the raw tag was shown on screen. The new opt-in
allowBasicHtmlTags option (default false) sanitizes both fields by
escaping everything and restoring only the exact, attribute-free
allowlisted tags, so the result is safe to render and arbitrary
HTML/script injection is impossible.
Adds unit tests for the sanitizer and an e2e test covering rendering and
an injection attempt.
Before screenshot: <img width="980" height="2726" alt="before"
src="https://github.com/user-attachments/assets/d1c871e1-21c5-44f9-ae40-da65c2c56f68"
/>
After screenshot: <img width="980" height="2726" alt="after"
src="https://github.com/user-attachments/assets/22d9e86b-221c-408e-a29b-718b0e98f236"
/>
> 3. Please run `node --run lint:prettier` before submitting so that
> style issues are fixed.
Done
**Note**: Sometimes the development moves very fast. It is highly
recommended that you update your branch of `develop` before creating a
pull request to send us your changes. This makes everyone's lives
easier (including yours) and helps us out on the development team.
Thanks again and have a nice day!
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
When a server responds with 304 (nothing changed since last fetch), the
response has no body. Several modules were trying to parse that empty
body anyway - which either cleared their cached data or threw an
exception. The result: a blank calendar, empty newsfeed, or missing
weather data after the next refresh cycle.
This was reported in the forum:
https://forum.magicmirror.builders/topic/20250/calendar-events-broadcasting-nothing-showing
The bug was "introduced" by #4120, which correctly started forwarding
304s to consumers - but not all were ready for it.
### Fix
Skip parsing on 304 and keep the existing data as-is:
- **calendar** - re-broadcasts cached events
- **newsfeed** - re-broadcasts cached items
- **buienradar, openmeteo, weatherflow, weathergov** - return early
before calling `response.json()`
This PR rewrites `module.js` to use a native ES6 `class` instead of
`Class.extend()` - the same old inheritance helper that was removed from
`node_helper.js` in #4147.
The normal module API stays the same: modules still use
`Module.register({...})`. Internally, `Module.create()` now creates a
named subclass for each module, copies over a cloned definition, and
only calls `init()` when it is actually a function.
Outcome: one less file in the browser bundle, no more magic `_super()`
wiring, better stack traces, and tests for the module creation path that
did not exist before. `module.js` is also now a plain class with no
external dependencies - an intentional step towards `export default
Module` when browser scripts eventually move to native ES modules.
Since these changes touch the core of how modules are created, I'd
appreciate a close review and any feedback on edge cases I may have
missed.
In PR #4072 GitHub Bot complained that `newsfeedfetcher.js` used the old
`.pipe()` method to connect streams (HTTP body → iconv decoding → RSS
parser). `.pipe()` has a weakness: errors in one stream are **not**
automatically forwarded to downstream streams. An I/O or decoding error
would silently disappear.
## Solution
Replaced `.pipe()` with `await stream.promises.pipeline()`. The
`pipeline()` API is designed to propagate errors correctly through the
entire chain and to clean up all streams on failure. Errors now reliably
land in the `catch` block and call `fetchFailedCallback` exactly once.
The redundant `parser.on("error")` handler was removed, as it would have
caught the same error again and called the callback a second time.
## Why not the bot's suggested fix?
The GitHub Bot suggested the older callback-based
`stream.pipeline(callback)` variant:
```js
stream.pipeline(nodeStream, iconv.decodeStream(...), parser, (error) => {
if (!error) return;
// error handling...
});
```
This has two drawbacks compared to my approach:
1. It uses the older callback style — `stream.promises.pipeline()` with
`async/await` is the modern, more readable API.
2. The bot's suggestion kept both the `parser.on("error")` handler
**and** the `catch` block, which would not have fixed the
double-callback problem.
----
Related to #4073
In PR #4072 GitHub Bot complained about an unused var. Instead of just
removing that one, I checked why ESLint hadn't complained about it: We
had disabled the rule for it.
So I enabled rule and resolved the issues that ESLint then detected.
Related to #4073
I was playing around with the newsfeed notification system
(`ARTICLE_MORE_DETAILS`, `ARTICLE_TOGGLE_FULL`, …) and discovered some
issues with the full article view:
The iframe was loading the CORS proxy URL instead of the actual article
URL, which could cause blank screens depending on the feed. Also, many
news sites block iframes entirely (`X-Frame-Options: DENY`) and the user
got no feedback at all — just an empty page. On top of that, scrolling
used `window.scrollTo()` which moved the entire MagicMirror page instead
of just the article.
This PR cleans that up:
- Use the raw article URL for the iframe (CORS proxy is only needed for
server-side feed fetching)
- Check `X-Frame-Options` / `Content-Security-Policy` headers
server-side before showing the iframe — if the site blocks it, show a
brief "Article cannot be displayed here." message and return to normal
view
- Show the iframe as a fixed full-screen overlay so other modules aren't
affected, scroll via `container.scrollTop`
- Keep the progressive disclosure behavior for `ARTICLE_MORE_DETAILS`
(title → description → iframe → scroll)
- Delete `fullarticle.njk`, replace with `getDom()` override
- Fix `ARTICLE_INFO_RESPONSE` returning proxy URL instead of real URL
- A few smaller fixes (negative scroll, null guard)
- Add `NEWSFEED_ARTICLE_UNAVAILABLE` translation to all 47 language
files
- Add e2e tests for the notification handlers (`ARTICLE_NEXT`,
`ARTICLE_PREVIOUS`, `ARTICLE_INFO_REQUEST`, `ARTICLE_LESS_DETAILS`)
## What this means for users
- The full article view now works reliably across different feeds
- If a news site blocks iframes, the user sees a brief message instead
of a blank screen
- Additional e2e tests make the module more robust and less likely to
break silently in future MagicMirror versions
This migrates the Newsfeed module to use the centralized HTTPFetcher
class (introduced in #4016), following the same pattern as the Calendar
module.
This continues the refactoring effort to centralize HTTP error handling
across all modules.
## Changes
**NewsfeedFetcher:**
- Refactored from function constructor to ES6 class (like the calendar
module in #3959)
- Replaced manual fetch() + timer handling with HTTPFetcher composition
- Uses structured error objects with translation keys
- Inherits smart retry strategies (401/403, 429, 5xx backoff)
- Inherits timeout handling (30s) and AbortController
**node_helper.js:**
- Updated error handler to use `errorInfo.translationKey`
- Simplified property access (`fetcher.url`, `fetcher.items`)
**Cleanup:**
- Removed `js/module_functions.js` (`scheduleTimer` no longer needed)
- Removed `#module_functions` import from package.json
## Related
Part of the HTTPFetcher migration effort started in #4016.
Next candidate: Weather module (client-side → server-side migration).
Since the project's inception, I've missed a clear separation between
default and third-party modules.
This increases complexity within the project (exclude `modules`, but not
`modules/default`), but the mixed use is particularly problematic in
Docker setups.
Therefore, with this pull request, I'm moving the default modules to a
different directory.
~~I've chosen `default/modules`, but I'm not bothered about it;
`defaultmodules` or something similar would work just as well.~~
Changed to `defaultmodules`.
Let me know if there's a majority in favor of this change.
