ubershmekel / redditp

Convert any reddit page to a presentation or slide show
https://redditp.com
MIT License
261 stars 102 forks source link

Fixing bug with encodedString is undefined #126

Closed MrSpiffyClean closed 3 years ago

MrSpiffyClean commented 3 years ago

I was browsing some nsfw subreddits when redditp reached a brick wall of sorts and didn't allow me to see any further images. A toast notification popped up stating "Redditp booboo: TypeError: encodedString is undefined"

After some digging, I believe to have found the underlying issue. Seaching for encodedString leads me to the decodeEntities function in Embedit.js, which is called with a reference to the Reddit JSON data. If the metadata doesn't contain an "u" key, the script ends up failing because undefined is passed as an argument and it breaks at the return of decodeEntities.

Checking the JSON of the subreddit in cause made me notice some keys (namely, "mp4" and "gif") that could probably be used for the same purpose as the "u" key so I coded this fix in relation to that. Haven't been able to test this though. Also, this doesn't guarantee that the script doesn't break if it finds something else that it doesn't understand/find.

ubershmekel commented 3 years ago

Is there a subreddit or reddit url which shows this issue?

MrSpiffyClean commented 3 years ago

While I am apprehensive in sharing the actual subreddit and post in question (this being a public record after all...), I was able to replicate the issue by creating a gallery that starts with a .gif file (I was unaware that video, as in .mp4 was not yet supported on Reddit galleries). You can find the post here: https://www.reddit.com/user/Tester_McGee_1234/comments/m2y645/test_gallery_for_redditp/ You are able to replicate by simply accessing https://www.redditp.com/u/Tester_McGee_1234. The JSON readout (https://www.reddit.com/user/Tester_McGee_1234.json) also confirms my thoughts.

This being said (and because I didn't know about the video support when I filed the request) if this moves forward, I would change "mp4" to "gif"; while this works insofar as not breaking the page, the actual video isn't rendered unless it reads the gif address. I have now been able to test a bit (by just replacing the code with the dev console) and it seems to work.

ubershmekel commented 3 years ago

Looking at https://deploy-preview-126--redditp.netlify.app/u/Tester_McGee_1234 I don't see the image that I see at https://www.reddit.com/user/Tester_McGee_1234

Though your fix did remove the Redditp booboo: Uncaught TypeError: Cannot read property 'replace' of undefined

MrSpiffyClean commented 3 years ago

I changed the key being accessed from mp4 to gif, according to my reasoning above. The current preview (https://deploy-preview-126--redditp.netlify.app/u/Tester_McGee_1234) now shows the gif.

ubershmekel commented 3 years ago

The gif files usually take longer to load and are of lower quality. Perhaps it should have come alongside

        pic.type = embedit.imageTypes.gifv;

Or something like that instead?

ubershmekel commented 3 years ago

By the way, thank you for this work!

MrSpiffyClean commented 3 years ago

The gif files usually take longer to load and are of lower quality. Perhaps it should have come alongside

        pic.type = embedit.imageTypes.gifv;

Or something like that instead?

I didn't quite understand what you meant by that, so I ended looking up how more of the code works. So, here's my reasoning. Looking in the source there is only one other place that references ".type", and that is the code that decides what kind of stuff is presenting (either image or video). So, you're using pic.type to signal the type of content and how to handle it. Fine. Changed back to the mp4 key, added the correct type, and the video didn't appear. The console log was logging of unsupported url, and that lead me to the converter list, so I added a new converter for the preview.redd.it address that the mp4 version of the file uses. It should now work with the mp4 showing.

ubershmekel commented 3 years ago

Thank you @MrSpiffyClean for this fix. I apologize the embedit.js code is a bit suboptimal and messy.

MrSpiffyClean commented 3 years ago

No problem, what ends up mattering is if it works in the end :)

An afterthought on this though. On the day you ended up publishing the changes I looked at some subreddits and the galleries were broken, which after some searching was the fault of reddit breaking something on their end, and not the changes. What seemed to break was the list that contains the order of the items in the gallery (which wasn't showing up on the json), not the actual links to the files. So, eventually, it could be considered that in the absence of the list, the galleries could still be shown, but with a warning about them not being in order. Something to think about in the future.