Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Files: video/image bytes are loaded twice on preview page #2217

Closed
lidel opened this issue Apr 8, 2024 · 2 comments · Fixed by #2254
Closed

Files: video/image bytes are loaded twice on preview page #2217

lidel opened this issue Apr 8, 2024 · 2 comments · Fixed by #2254
Assignees
@lidel
Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization released

Comments

@lidel
Copy link
Member

lidel commented Apr 8, 2024 •
edited
Loading

Tested on webui.ipfs.io and http://127.0.0.1:5001/webui/, Firefox 124.0.2 and Google Chrome 124.0.6367.18.

Opening image in Files triggers fetch twice

  • once via Kubo RPC and /api/v0/cat
  • once via subdomain gateway at cid.ipfs.localhost:port

2024-04-09-004847_1475x68_scrot

I guess we had code somewhere which should try one, and then fallback to other, but instead does both?

There is also unrelated(?) bug in Chrome where the gateway GET listed here fails (#2218)
@lidel lidel added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Apr 8, 2024
@lidel
Copy link
Member Author

lidel commented Aug 30, 2024 •
edited
Loading

#2218 is resolved, but this one is still the case in v4.3.0.

This is extra bad for videos, which are fetched entirely:

2024-08-31_00-31

I think the problem is read() defined here:

and then used in FilePreview.js ONLY for inspecting plain text CIDs.

If you add a big text file, only small chunk is rendered, and then you can click "load more" to read the rest, but the browser already fetched the thing via /api/v0/cat.

We could fix this, by doing cat with offset and length, but I think we should just remove this complexity, it only leads to perf. bugs like this one, when different people refactor code across the years.

I propose:

  • remove read: () => ipfs.cat(stats.cid) from src/bundles/files/actions.js and loadContent from src/files/file-preview/FilePreview.js
  • replace t(loadMore) Button with "preview the entire file on gateway" based on already existing option from cantPreview

I will open PR with fix later today.

@lidel lidel changed the title Files: image bytes are loaded twice Files: video/image bytes are loaded twice on preview page Aug 30, 2024
@lidel lidel self-assigned this Aug 30, 2024
lidel added a commit that referenced this issue Aug 31, 2024
@lidel
fix: avoid duplicated fetch during preview
a6170ce 
Closes #2217
@lidel lidel mentioned this issue Aug 31, 2024
Merged
3 tasks
@lidel lidel closed this as completed in eefae25 Sep 3, 2024
ipfs-gui-bot pushed a commit that referenced this issue Sep 23, 2024
@semantic-release-bot
chore(release): 4.3.1 [skip ci]
eb3076b 
## [4.3.1](v4.3.0...v4.3.1) (2024-09-23)

 CID `bafybeideglc722hiwhsy4kiyl2fivf5lr6wozy2iuixtgzkvl3v4hasaty`

 ---

### Bug Fixes

* **file-preview:** safeSubresourceGwUrl ([#2253](#2253)) ([bb861a3](bb861a3)), closes [/github.com//issues/2246#issuecomment-2322192398](https://github.com/ipfs//github.com/ipfs/ipfs-webui/issues/2246/issues/issuecomment-2322192398)
* **files:** avoid duplicated fetch during preview ([#2254](#2254)) ([eefae25](eefae25)), closes [#2217](#2217)
* **files:** prefer subdomain gw in copied share links ([#2255](#2255)) ([e8c4421](e8c4421))

### Trivial Changes

* .io → .tech ([b9f622d](b9f622d))
* browserslist@latest ([#2251](#2251)) ([809c55a](809c55a))
* Pull transifex translations ([#2258](#2258)) ([2250374](2250374))
* use [email protected] ([#2257](#2257)) ([99ba9ac](99ba9ac))
@ipfs-gui-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 4.3.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lidel lidel

Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization released
Projects
No open projects
IPFS-GUI (PL EngRes)
Status: No status
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(files): avoid duplicated fetch during preview ipfs/ipfs-webui
2 participants
@lidel @ipfs-gui-bot