QuickShare: Fix attempt for Plasma freezing when trying to paste large images to imgur
AbandonedPublic

Authored by yuenlim on Mar 5 2017, 6:23 PM.

Details

Reviewers
apol
Group Reviewers
Plasma
Summary

Oddly, when pasting raw image data into QuickShare -> imgur, if the image data is sufficiently large KIO::storedHttpPost() seems to trigger a notification that reads:

"Transferring: Finished" "data:image/png;base64, <the entire raw data of the image to paste>"

This causes Plasma to basically freeze, presumably because the notification system isn't meant to handle megabytes of text.

This strangely doesn't happen (the notification does not appear to get triggered) if the image is not too large.

I am able to reliably reproduce the notification attempt + freeze with a 3000x2000 Spectacle screenshot with a sufficiently complex wallpaper, and reliably reproduce the non-buggy no notification case using smaller rectangular region screenshots.

I have also found that I can "fix" the large image case by simply omitting the KIO::HideProgressInfo flag from the KIO::storedHttpPost() call in the imgur plugin code. This seems to remove the notification + freeze with no ill effects.

Wondering if this might be an acceptable patch to go in, and if someone can shed some light on why I'm seeing such odd behavior :)

Test Plan

Can be reproduced with a 3000x2000 display and using a blank desktop with the "Evening Glow by Ian Hex" wallpaper.

  1. Take a screenshot of the whole blank desktop with Spectacle
  2. Click "Copy to Clipboard"
  3. Right click QuickShare plasmoid > Paste
  4. Select Imgur action

This should cause plasma to become unresponsive, shell output should indicate the notification as in description.

After applying the changes in the patch, plasma does not become unresponsive, no notification happens, and the paste succeeds.

Diff Detail

Repository
R495 Purpose Library
Lint
Lint Skipped
Unit
Unit Tests Skipped
yuenlim created this revision.Mar 5 2017, 6:23 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 5 2017, 6:23 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik added a subscriber: broulik.Mar 5 2017, 6:31 PM

The notification is emitted by the notification applet itself and contains the destination URL of a job that finished. Maybe the applet should truncate the text? Or maybe KIO should do that? It's a http POST after all which I don't see why it should have a data url as target.

yuenlim edited the test plan for this revision. (Show Details)Mar 5 2017, 6:37 PM
apol edited edge metadata.Mar 5 2017, 11:56 PM

The notification is emitted by the notification applet itself and contains the destination URL of a job that finished. Maybe the applet should truncate the text? Or maybe KIO should do that? It's a http POST after all which I don't see why it should have a data url as target.

+1

mart added a subscriber: mart.Mar 6 2017, 1:43 PM

The notification is emitted by the notification applet itself and contains the destination URL of a job that finished. Maybe the applet should truncate the text? Or maybe KIO should do that? It's a http POST after all which I don't see why it should have a data url as target.

+1

Got it. I'll try digging into the notification applet/KIO code and see.

yuenlim abandoned this revision.Mar 12 2017, 5:33 PM

Fixed in D5013