Streams_Stream::metas(): fall back to Streams/icon sizes, not Streams/image - #37
Open
zattak1 wants to merge 1 commit into
Open
Streams_Stream::metas(): fall back to Streams/icon sizes, not Streams/image#37zattak1 wants to merge 1 commit into
zattak1 wants to merge 1 commit into
Conversation
Q_Image::getSizes() throws for any stream type with no Q/images/<type> entry, which includes Streams/chat, so Streams_Stream::metas() takes its catch branch for every conversation. That branch fell back to the "Streams/image" size list, whose largest entry is "1000x" => "1000x.jpg". Stream icons are written by Streams::importIcon(), whose $save default is "Streams/icon", whose largest entry is "1000" => "1000.jpg". Reader and writer were reading two different config keys one character apart, so every conversation permalink advertised an og:image / twitter:image that 404ed while the real 1000.jpg sat beside it on disk. Falling back to the writer's own default makes the two agree. Types that DO have their own images config (Calendars/event) are unaffected -- they never reach this branch, which is why event permalinks already unfurled correctly and conversations did not. Refs ro#388
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
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.
Stream permalinks advertise an
og:image/twitter:imagethat 404s, for every stream type that has noQ/images/<type>config of its own — which includesStreams/chat, so every conversation permalink is affected.Cause
Streams_Stream::metas():Q_Image::getSizes()opens withQ_Config::expect("Q","images",$type,'sizes'), which throws when the type has no images entry — so forStreams/chatthecatchbranch is not an edge case, it is the path every request takes.The two size lists in
Streams/config/plugin.json:end()gives1000.jpgon the first and1000x.jpgon the second. But the files are written byStreams::importIcon($publisherId, $streamName, $sources, $save = "Streams/icon", …)— the first list. So the writer and the reader name two different config keys, and the meta tag asks for a file that was never generated.Observed on a live install: the advertised
…/icon/<time>/1000x.jpgreturns 404 while its sibling…/icon/<time>/1000.jpgreturns 200 with the real image.40,50and80resolve identically under either key, which is why every other use of the icon looks correct and only link previews break.By contrast a type that does carry an images config (
Calendars/event) takes thetrybranch and unfurls correctly — the symptom tracks the presence of a config entry, not the data.Fix
Fall back to
Streams/icon, the listimportIcon()writes with, so reader and writer agree. One word, no behaviour change for any type that has its own images config.