Millisecond glitch on image upload

[fredck]

📝 Provide detailed reproduction steps (if any)

1. In Firefox, go to the base64 upload doc demo.
2. Upload an image (easer to see with a small one).

✔️ Expected result

Image "uploaded" with no glitches.

❌ Actual result

A perceivable glitch happens.

📃 Other details

• Browser: Firefox
• OS: Mac
• CKEditor version: 20.0.0

I wasn't able to reproduce the problem in Chrome, but it has been reported in GitHub Writer that a similar issue happens in Chrome as well: ckeditor/github-writer#203.

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[Reinmar]

@fredck, could you test whether this issue occurs when WidgetTypeAround is disabled (you'd need to remove it from the Widget plugin deps)

[fredck]

@fredck, could you test whether this issue occurs when WidgetTypeAround is disabled (you'd need to remove it from the Widget plugin deps)

I've just tried it with a local build of GitHub Writer as it suffers with this issue (ckeditor/github-writer#203). Removing that plugin didn't change anything and the glitch is still there. I confirmed that the plugin is disabled because there was no fancy arrows and horizontal carets anymore around the image.

I wasn't able to test it with the base64 example, as described in this issue, but I believe you guys should be able to easily do so. Thanks!

[Reinmar]

Seems to be related to #7633.

[Reinmar]

#7633 makes it quite severe.

[oleq]

FYI: #7633 is a different issue and there's a PR addressing it.

[Reinmar]

Thanks for debugging this, @oleq

What worries me, though, is that two similar issues appeared shortly one after another. It's either Chrome's issue or we changed something and it'd be good to know what was that.

Also, besides image upload I can see that normal image paste (image copied from the editor) is affected too. The resize handles are repositioned with a long delay, even 300ms-500ms. This definitely didn't happen before. And as Fred checked, this is not related to WidgetTypeAround. You can even observe the glitch on this gif:

It'd be good to know WTH is happening there. Bisecting it might actually help a lot.

[oleq]

@fredck Can you post some GIFs? I'm having trouble reproducing your issue. It's either too subtle or there are some external factors involved. Thanks!

[Reinmar]

I also can't reproduce the issue anymore. I saw it previously.

[panr]

There's a 200ms throttle on redrawing the handles after you click on the image - https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-widget/src/widgetresize.js#L90

I had a pretty straightforward fix for this 6c2a01c but after Olek's changes I decided to remove it (also because it was related to the config.image.disableResizeHandles which we decided to get rid of the scope of the ticket).

[Reinmar]

There should be no delay when the user focuses the image. Throttling makes sense when something happens very frequently. I remember that we added it for performance reasons but that was related to the performance during resizing. Changing how it behaves on image focus must've been coincidental.

Anyway, I'm for changing this – on focus the redraw should be immediate.

[Reinmar]

@oleq pointed out that we can also think about reconfiguring throttle to execute on the leading edge. That'd shorten the initial reaction.

[Reinmar]

So, we have two possible solutions:

• Better (easier): reconfigure throttle.
• Slightly more complicated: force refresh on view render.

[fredck]

Can you post some GIFs? I'm having trouble reproducing your issue. It's either too subtle or there are some external factors involved. Thanks!

Was finally able to do so... Chrome:

Firefox:

This was tested with GitHub Writer after upgrading it to CKEditor 5 v21.0.0.

[niegowski]

This glitch is also happening on the linked images - when the user is removing a link from the image.

[panr]

Unfortunately, I couldn’t reproduce exactly the same glitch on Chrome / Firefox. But I tried to follow the whole process step by step with devtools and debugger and I presume that the issue may be related to the reading status of the file upload. During this stage, an image placeholder is created, but its styling depends on those styles:

This means that the placeholder will expand to the whole width of the editor, no matter how wide / big the uploading image is. When we upload a smaller (and "lighter") image — like 200px or so — the placeholder flashes for a moment (with that blue outline and height equals 0 — BUT the placeholder contains .ck-upload-placeholder-loader which is this empty upload placeholder svg file — https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-image/theme/icons/image_placeholder.svg) and disappears.

Then the uploaded image appears instead.

Both, placeholder and actual image have different widths and heights and both have outlines, so it looks like the image stretches and shrinks within a few milliseconds.

EDIT:

Splitting the gifs above, we can see that it's not a progress bar, but the outline itself (don't know why the placeholder has no height here):

Firefox:

Chrome (I don't understand what happened here):

cc @oleq

[oleq]

Thanks, @panr. 

My two cents:

Blame

I searched git blame and I found out this width: 100% was introduced along with this PR ckeditor/ckeditor5-theme-lark#188 and addressed this issue #5150. Unfortunately, this does not help much.

Code

I also briefly checked the code and it looks like the part responsible for the "glitch" in FF is somewhere in here

ckeditor5/packages/ckeditor5-image/src/imageupload/imageuploadprogress.js

Lines 85 to 104 in bcf3af6

	if ( status == 'uploading' ) {
	const loader = fileRepository.loaders.get( uploadId );
	// Start appear effect if needed - see https://github.com/ckeditor/ckeditor5-image/issues/191.
	_startAppearEffect( viewFigure, viewWriter );
	if ( !loader ) {
	// There is no loader associated with uploadId - this means that image came from external changes.
	// In such cases we still want to show the placeholder until image is fully uploaded.
	// Show placeholder if needed - see https://github.com/ckeditor/ckeditor5-image/issues/191.
	_showPlaceholder( placeholder, viewFigure, viewWriter );
	} else {
	// Hide placeholder and initialize progress bar showing upload progress.
	_hidePlaceholder( viewFigure, viewWriter );
	_showProgressBar( viewFigure, viewWriter, loader, editor.editing.view );
	_displayLocalImage( viewFigure, viewWriter, loader );
	}
	return;
	}

I started commenting out some of the helpers _hidePlaceholder(), _showProgressBar(), _displayLocalImage() and without them, it looks smooth. So I suppose The Glitch could be caused by many things happening at once because the image is very small and Firefox is quick to read it and upload it (but not as quick as Chrome). 

Recordings

Finally, I decided to capture The Glitch myself. First, I recorded the GIF below in the easy-image manual test on Firefox in 50fps and slowed down to 25%:

And here's the same scenario in the Github Writer Firefox addon (also in slow-mo):

Note there's a difference between vanilla CKEditor 5 and GHWriter. In the former, the image never stretches to 100% at any point. In the latter, there's a clear moment when the outline takes 100% width of the editing root (it has height, though, unlike the screenshot from the @panr comment above). Which brings a couple of questions:

• Has the code of GHW or CKEditor 5 changed over the last ~month so much that the height that was 0px (Radek's screenshot) is now non-zero (my last GIF ☝️ )?
• Maybe it all comes down to specific styles used in GHW? As I mentioned, in the vanilla CKEditor 5, the image never stretches horizontally at any point, so maybe this is the reason why users perceive this as a glitch (it does look like one). @fredck, can you check this out and see if getting rid of these styles brings the UX back to the expected level?

[panr]

in the vanilla CKEditor 5, the image never stretches horizontally at any point

Hmm... are you sure? I caught this yesterday in imageuploadviaurl.html manual test 🤔 The image stretches (specifically image placeholder) in the screen below:
https://user-images.githubusercontent.com/1303365/92606924-cc5df980-f2b3-11ea-8e7d-d8789bdcee83.png

And today I made these frames to show how I see the process in my editor (Chrome):

---

I also briefly checked the code and it looks like the part responsible for the "glitch" in FF is somewhere in here

I bet that the glitch is somewhere in reading state ;-D

Maybe we should wait a little bit longer and get the image data (width and height) and then show a placeholder that mimics the actual (fixed) image size?

[mlewand]

@oleq , please double check the width aspect. @panr is right in saying that it also occurs in vanilla CKE5.

Disclaimer, poor screenshot resolution because… chrome devtools work on such potato screens (make sense from perf perspective).

tl;dr;

• UX is worse when uploading an image vs inserting via URL
• There's an oversized empty widget outline during upload
• You may ask: "But on @fredck screencast there's no wrapper outline, that's something different 😠👿" - our wrapper outline/border fades in. So my assumption is that due to low framerate of gif it picked up a frame from the start

Based on imageuploadviaurl manual test

Image upload (chrome)

4 frames are produced (2 reflows + 2 repaints)

Empty widget container is inserted (no spinner visible) Container seems to be much larger than it should be balloon is mispositioned	Balloon panel position is fixed Spinner is visible Caption fades in (I believe it's available from the beginning, it's just the animation that makes it visible here)	Image is loaded and visible Ballon panel is mispositioned again	Balloon is being moved to the final position

Image insert by URL

Following frames are produced:

The text in image wrapper is "Enter image caption"	Balloon panel position is fixed

Fredck case

Here are key frames from fredck screencast:

First, the editable gets much bigger, to shrink later on. I assume it's, again, caused by widget wrapper.

Notes regarding previous comments

I started commenting out some of the helpers _hidePlaceholder(), _showProgressBar(), _displayLocalImage() and without them

ckeditor5/packages/ckeditor5-image/src/imageupload/imageuploadprogress.js

Lines 85 to 104 in bcf3af6

	if ( status == 'uploading' ) {
	const loader = fileRepository.loaders.get( uploadId );
	// Start appear effect if needed - see https://github.com/ckeditor/ckeditor5-image/issues/191.
	_startAppearEffect( viewFigure, viewWriter );
	if ( !loader ) {
	// There is no loader associated with uploadId - this means that image came from external changes.
	// In such cases we still want to show the placeholder until image is fully uploaded.
	// Show placeholder if needed - see https://github.com/ckeditor/ckeditor5-image/issues/191.
	_showPlaceholder( placeholder, viewFigure, viewWriter );
	} else {
	// Hide placeholder and initialize progress bar showing upload progress.
	_hidePlaceholder( viewFigure, viewWriter );
	_showProgressBar( viewFigure, viewWriter, loader, editor.editing.view );
	_displayLocalImage( viewFigure, viewWriter, loader );
	}
	return;
	}

I wonder if that was really the case, as the only thing that I was able to comment out without breaking the feature was _showProgressBar and with it commented the issue persists. Skipping _displayLocalImage() keeps us with a placeholder spinner instead of an image, while skipping _hidePlaceholder() leaves the spinner on top of an image.

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).