fix: [Widget Image] Remove oldurl string check since it's always a string and prevent the ObjectURL cleaning resulting in a memory leak.

[joseph2rs]

See #3170

[github-actions]

👈 Launch a binder notebook on branch joseph2rs/ipywidgets/widget_image_fix_revoke_object_url

[jasongrout]

Thanks!

This check was added in 1b16391 (#1676) to distinguish between that src being an external url or an internal object url.

However, it looks like step 2 of the revoking process checks to see if the url has scheme blob:, so I think we can call revoke on any URL (including external URLs) and it will silently return if the url is not a blob url. Indeed, the standard says:

Note: This means that rather than throwing some kind of error, attempting to revoke a URL that isn’t registered will silently fail. User agents might display a message on the error console if this happens.

I see that we have this same logic in the video and audio widgets. Can you fix it in those files too?

[jasongrout]

FYI, on current firefox and chrome, URL.revokeObjectURL('http://google.com') just silently returns without an error message, so I think we don't even have the error message the standard warns may happen.

[jasongrout]

Other places are at

ipywidgets/packages/controls/src/widget_audio.ts

Lines 65 to 67 in 1a12162

	if (oldurl && typeof oldurl !== 'string') {
	URL.revokeObjectURL(oldurl);
	}

and

ipywidgets/packages/controls/src/widget_video.ts

Lines 68 to 70 in 1a12162

	if (oldurl && typeof oldurl !== 'string') {
	URL.revokeObjectURL(oldurl);
	}

[joseph2rs]

FYI i removed also the check on oldurl since passing undefined or null fail silently.

[jasongrout]

FYI i removed also the check on oldurl since passing undefined or null fail silently.

Reading the standard, we aren't guaranteed passing in undefined or null will always fail silently, just that a url that is not an object url will. In the standard, the first step is to parse the url, and the parsing standard seems to really want a string.

Can you put back in the oldurl test so we're following the standard more closely? That will make it more certain it will work across browsers, at least ones that implement the spec as written.

[jasongrout]

Thanks!