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.

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

Add reload-failed-proxied-images feature #2393

Merged
merged 19 commits into from Sep 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Expand Up @@ -21,13 +21,15 @@
"content-scripts-register-polyfill": "^1.0.0",
"copy-text-to-clipboard": "^2.0.0",
"debounce-fn": "^3.0.1",
"delay": "^4.3.0",
"delegate-it": "^1.1.0-7",
"dom-chef": "^3.6.1",
"dom-loaded": "^1.1.0",
"doma": "^1.0.3",
"element-ready": "^4.1.0",
"fit-textarea": "^1.1.0",
"github-reserved-names": "^1.1.5",
"image-promise": "^6.0.2",
"indent-textarea": "^1.0.4",
"insert-text-textarea": "^1.0.3",
"intervalometer": "^1.0.5",
Expand Down
1 change: 1 addition & 0 deletions readme.md
Expand Up @@ -137,6 +137,7 @@ Thanks for contributing! 🦋🙌
- [](# "swap-branches-on-compare") [Adds link to swap branches in the branch compare view.](https://user-images.githubusercontent.com/857700/42854438-821096f2-8a01-11e8-8752-76f7563b5e18.png)
- [](# "linkify-branch-refs") [Linkifies branch references in "Quick PR" pages.](https://user-images.githubusercontent.com/1402241/30208043-fa1ceaec-94bb-11e7-9c32-feabcf7db296.png)
- [](# "hide-zero-packages") [Hides the `Packages` tab in repositories if it’s empty.](https://user-images.githubusercontent.com/35382021/62426530-688ef780-b6d5-11e9-93f2-515110aed1eb.jpg)
- [](# "reload-failed-proxied-images") [Retries downloading images that failed downloading due to GitHub limited proxying.](https://user-images.githubusercontent.com/14858959/64068746-21991100-cc45-11e9-844e-827f5ac9b51e.png)

<!-- Refer to style guide above. Keep this message between sections. -->

Expand Down
1 change: 1 addition & 0 deletions source/content.ts
Expand Up @@ -136,6 +136,7 @@ import './features/linkify-symbolic-links';
import './features/hide-zero-packages';
import './features/revert-file';
import './features/hidden-review-comments-indicator';
import './features/reload-failed-proxied-images';
import './features/clean-rich-text-editor';

// Add global for easier debugging
Expand Down
26 changes: 26 additions & 0 deletions source/features/reload-failed-proxied-images.tsx
@@ -0,0 +1,26 @@
import delay from 'delay';
import delegate from 'delegate-it';
import loadImage from 'image-promise';
import features from '../libs/features';

async function handleErroredImage({target}: any): Promise<void> {
await delay(5000);
try {
// A clone image retries downloading
// `loadImage` awaits it
// If successfully loaded, the failed image will be replaced.
target.replaceWith(await loadImage(target.cloneNode() as HTMLImageElement));
} catch {}
}

function init(): void {
delegate('img[src^="https://camo.githubusercontent.com/"]', 'error', handleErroredImage, true);
}
Copy link
Member

@fregante fregante Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an additional request will not cause that much trouble assuming that the image is cached and ready to be served.

Let's retry once, then.

If you use https://github.com/fregante/image-promise this whole feature can just be:

async function handleErroredImage({target}): Promise<void> {
	await delay(5000);
	try {
		// A cloned image retries downloading
		// `loadImage` awaits it
		// If successfully loaded, the failed image will be replaced.
		target.replaceWith(await loadImage(target.cloneNode()));
	} catch {}
}

function init(): void {
	delegate('img[src^="https://camo.githubusercontent.com"]', 'error', handleErroredImage);
}

features.add({
	id: __featureName__,
	description: 'Retries downloading images that failed downloading due to GitHub limited proxying.',
	screenshot: false,
	init
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my tests/debugging delegate does not work in this case. I think GitHub gabs the errors for statistics and the event does not bubble up to the body element. Maybe I am wrong.

Copy link
Member

@fregante fregante Aug 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works if you pass a true as the last parameter of delegate to make it a capture listener.


features.add({
id: __featureName__,
description: 'Retries downloading images that failed downloading due to GitHub limited proxying.',
screenshot: 'https://user-images.githubusercontent.com/14858959/64068746-21991100-cc45-11e9-844e-827f5ac9b51e.png',
load: features.onAjaxedPages,
init
});