Skip to content

fix(newsfeed): fix full article view and add framing check#4039

Merged
khassel merged 3 commits intoMagicMirrorOrg:developfrom
KristjanESPERANTO:newsfeed
Feb 28, 2026
Merged

fix(newsfeed): fix full article view and add framing check#4039
khassel merged 3 commits intoMagicMirrorOrg:developfrom
KristjanESPERANTO:newsfeed

Conversation

@KristjanESPERANTO
Copy link
Copy Markdown
Collaborator

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

The iframe src was built using getActiveItemURL() which prepends the
CORS proxy prefix (http://localhost:8080/cors?url=...). Browsers refuse
to load that URL in an iframe, resulting in 403/500 errors with no
visible feedback to the user.

Changes:
- Delete fullarticle.njk (3-line file, replaced by getDom() override)
- Add getDom() override that renders loading/unavailable/iframe states
  directly via DOM API, using the raw article URL for the iframe src
- Add server-side framing check in node_helper: sends a HEAD request
  and inspects X-Frame-Options / Content-Security-Policy headers before
  showing the iframe; sends ARTICLE_URL_STATUS socket notification back
- When an article cannot be framed: show NEWSFEED_ARTICLE_UNAVAILABLE
  message for 3 seconds, then auto-return to normal newsfeed view
- Replace window.scrollTo() with container.scrollTop on an
  overflow-y:auto container � prevents scrolling other modules out of view
- iframe is now a position:fixed full-screen overlay (z-index:1000) so
  other module positions are unaffected
- Simplify showFullArticle(): always opens iframe directly; guard
  against missing article URL
- Implement ARTICLE_MORE_DETAILS as progressive disclosure:
  title → description → iframe → scroll down
- Fix ARTICLE_INFO_RESPONSE returning CORS proxy URL instead of raw URL
- Fix ARTICLE_SCROLL_UP allowing negative scrollPosition
- Add null guard in ARTICLE_INFO_REQUEST handler to prevent crash when
  newsItems not yet loaded
Add the new key to all 47 language files. The message is shown for
3 seconds when a news article cannot be displayed in an iframe (e.g.
because the site sends X-Frame-Options: DENY).

Also fix ps.json: add missing NEWSFEED_NO_ITEMS key and correct
indentation of UPDATE_INFO_MULTIPLE.
Add a dedicated 'Newsfeed module > Notifications' describe block with
tests for: ARTICLE_NEXT, ARTICLE_PREVIOUS (including wrap-around in
both directions), ARTICLE_INFO_REQUEST (verifying response payload and
that the URL is the raw article URL, not a CORS proxy URL), and
ARTICLE_LESS_DETAILS.

Add tests/configs/modules/newsfeed/notifications.js with a long
updateInterval (1 hour) to prevent auto-rotation during tests.
@sdetweil
Copy link
Copy Markdown
Collaborator

Nice work.
one investigative question

why not use the jdk file?

Supposedly it’s as functional as direct dom.
just a question. I have used similar functionality via Angular

@KristjanESPERANTO
Copy link
Copy Markdown
Collaborator Author

why not use the jdk file?

The full article view now has an async step (server checks headers before showing the iframe), so the DOM needs to react to a callback. With a template you'd render it and then immediately manipulate the result via querySelector anyway — at that point the template doesn't add much. The old one was only 3 lines, so moving it into getDom() felt simpler.

@rejas
Copy link
Copy Markdown
Collaborator

rejas commented Feb 28, 2026

No complains from my side other then the question if the translations were done with AI?

@KristjanESPERANTO
Copy link
Copy Markdown
Collaborator Author

KristjanESPERANTO commented Feb 28, 2026

if the translations were done with AI?

Yes, I used Gemini for the translation. I will try to include this information in future PR texts.

@khassel khassel merged commit df8a882 into MagicMirrorOrg:develop Feb 28, 2026
9 checks passed
@KristjanESPERANTO KristjanESPERANTO deleted the newsfeed branch March 1, 2026 00:24
@khassel khassel mentioned this pull request Apr 1, 2026
khassel added a commit that referenced this pull request Apr 1, 2026
## Release Notes
Thanks to: @angeldeejay, @in-voker, @JHWelch, @khassel,
@KristjanESPERANTO, @rejas, @sdetweil
> ⚠️ This release needs nodejs version >=22.21.1 <23 || >=24 (no change
to previous release)

[Compare to previous Release
v2.34.0](v2.34.0...v2.25.0)

> ⚠️ We introduced some internal changes with this release, please read
[this forum
post](https://forum.magicmirror.builders/topic/20138/upcoming-release-april-1-2026-breaking-changes-some-operational-changes)
before upgrading!

### [core]
- Prepare Release 2.35.0 (#4071)
- docs: add security policy and vulnerability reporting guidelines
(#4069)
- refactor: simplify internal `require()` calls (#4056)
- allow environment variables in cors urls (#4033)
- fix cors proxy getting binary data (e.g. png, webp) (#4030)
- fix: correct secret redaction and optimize loadConfig (#4031)
- change loading config.js, allow variables in config.js and try to
protect sensitive data (#4029)
- remove kioskmode (#4027)
- Add dark theme logo (#4026)
- move custom.css from css to config (#4020)
- move default modules from /modules/default to /defaultmodules (#4019)
- update node versions in workflows (#4018)
- [core] refactor: extract and centralize HTTP fetcher (#4016)
- fix systeminformation not displaying electron version (#4012)
- Update node-ical and support it's rrule-temporal changes (#4010)
- Change default start scripts from X11 to Wayland (#4011)
- refactor: unify favicon for index.html and Electron (#4006)
- [core] run systeminformation in subprocess so the info is always
displayed (#4002)
- set next release dev number (#4000)

### [dependencies]
- update dependencies (#4068)
- update dependencies incl. electron to v41 (#4058)
- chore: upgrade ESLint to v10 and fix newly surfaced issues (#4057)
- chore: update ESLint and plugins, simplify config, apply new rules
(#4052)
- chore: update dependencies + add exports, files, and sideEffects
fields to package.json (#4040)
- [core] refactor: enable ESLint rule require-await and handle detected
issues (#4038)
- Update node-ical and other deps (#4025)
- chore: update dependencies (#4021)
- chore(eslint): migrate from eslint-plugin-vitest to
@vitest/eslint-plugin and run rules only on test files (#4014)
- Update deps as requested by dependabot (#4008)
- update Collaboration.md and dependencies (#4001)

### [logging]
- refactor: further logger clean-up (#4050)
- Fix Node.js v25 logging prefix and modernize logger (#4049)

### [modules/calendar]
- fix(calendar): make showEnd behavior more consistent across time
formats (#4059)
- test(calendar): fix hardcoded date in event shape test (#4055)
- [calendar] refactor: delegate event expansion to node-ical's
expandRecurringEvent (#4047)
- calendar.js: remove useless hasCalendarURL function (#4028)
- fix(calendar): update to node-ical 0.23.1 and fix full-day recurrence
lookup (#4013)
- fix(calendar): correct day-of-week for full-day recurring events
across all timezones (#4004)

### [modules/newsfeed]
- fix(newsfeed): fix full article view and add framing check (#4039)
- [newsfeed] refactor: migrate to centralized HTTPFetcher (#4023)

### [modules/weather]
- fix(weather): fix openmeteo forecast stuck in the past (#4064)
- fix(weather): fix weathergov forecast day labels off by one (#4065)
- weather: fixes for templates (#4054)
- weather: add possibility to override njk's and css (#4051)
- Use getDateString in openmeteo (#4046)
- [weather] refactor: migrate to server-side providers with centralized
HTTPFetcher (#4032)
- [weather] feat: add Weather API Provider  (#4036)

### [testing]
- chore: remove obsolete Jest config and unit test global setup (#4044)
- replace template_spec test with config_variables test (#4034)
- refactor(clientonly): modernize code structure and add comprehensive
tests (#4022)
- Switch to undici Agent for HTTPS requests (#4015)
- chore: migrate CI workflows to ubuntu-slim for faster startup times
(#4007)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
Co-authored-by: Bugsounet - Cédric <github@bugsounet.fr>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: sam detweiler <sdetweil@gmail.com>
Co-authored-by: Veeck <github@veeck.de>
Co-authored-by: veeck <gitkraken@veeck.de>
Co-authored-by: Magnus <34011212+MagMar94@users.noreply.github.com>
Co-authored-by: Ikko Eltociear Ashimine <eltociear@gmail.com>
Co-authored-by: DevIncomin <56730075+Developer-Incoming@users.noreply.github.com>
Co-authored-by: Nathan <n8nyoung@gmail.com>
Co-authored-by: mixasgr <mixasgr@users.noreply.github.com>
Co-authored-by: Savvas Adamtziloglou <savvas-gr@greeklug.gr>
Co-authored-by: Konstantinos <geraki@gmail.com>
Co-authored-by: OWL4C <124401812+OWL4C@users.noreply.github.com>
Co-authored-by: BugHaver <43462320+bughaver@users.noreply.github.com>
Co-authored-by: BugHaver <43462320+lsaadeh@users.noreply.github.com>
Co-authored-by: Koen Konst <koenspero@gmail.com>
Co-authored-by: Koen Konst <c.h.konst@avisi.nl>
Co-authored-by: dathbe <github@beffa.us>
Co-authored-by: Marcel <m-idler@users.noreply.github.com>
Co-authored-by: Kevin G. <crazylegstoo@gmail.com>
Co-authored-by: Jboucly <33218155+jboucly@users.noreply.github.com>
Co-authored-by: Jboucly <contact@jboucly.fr>
Co-authored-by: Jarno <54169345+jarnoml@users.noreply.github.com>
Co-authored-by: Jordan Welch <JordanHWelch@gmail.com>
Co-authored-by: Blackspirits <blackspirits@gmail.com>
Co-authored-by: Samed Ozdemir <samed@xsor.io>
Co-authored-by: in-voker <58696565+in-voker@users.noreply.github.com>
Co-authored-by: Andrés Vanegas Jiménez <142350+angeldeejay@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants