fix(newsfeed): fix full article view and add framing check#4039
Open
KristjanESPERANTO wants to merge 3 commits intoMagicMirrorOrg:developfrom
Open
fix(newsfeed): fix full article view and add framing check#4039KristjanESPERANTO wants to merge 3 commits intoMagicMirrorOrg:developfrom
KristjanESPERANTO wants to merge 3 commits intoMagicMirrorOrg:developfrom
Conversation
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.
Collaborator
|
Nice work. why not use the jdk file? Supposedly it’s as functional as direct dom. |
khassel
approved these changes
Feb 27, 2026
Collaborator
Author
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. |
Collaborator
|
No complains from my side other then the question if the translations were done with AI? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 usedwindow.scrollTo()which moved the entire MagicMirror page instead of just the article.This PR cleans that up:
X-Frame-Options/Content-Security-Policyheaders server-side before showing the iframe — if the site blocks it, show a brief "Article cannot be displayed here." message and return to normal viewcontainer.scrollTopARTICLE_MORE_DETAILS(title → description → iframe → scroll)fullarticle.njk, replace withgetDom()overrideARTICLE_INFO_RESPONSEreturning proxy URL instead of real URLNEWSFEED_ARTICLE_UNAVAILABLEtranslation to all 47 language filesARTICLE_NEXT,ARTICLE_PREVIOUS,ARTICLE_INFO_REQUEST,ARTICLE_LESS_DETAILS)What this means for users