Page MenuHomePhabricator

Reduce thumbnail flickering in comment previews
ClosedPublic

Authored by epriestley on Apr 6 2016, 10:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 9, 5:03 AM
Unknown Object (File)
Mon, Apr 1, 6:24 AM
Unknown Object (File)
Mar 16 2024, 6:23 PM
Unknown Object (File)
Mar 16 2024, 4:43 PM
Unknown Object (File)
Mar 16 2024, 4:08 PM
Unknown Object (File)
Mar 16 2024, 4:03 PM
Unknown Object (File)
Mar 16 2024, 3:44 PM
Unknown Object (File)
Mar 13 2024, 7:16 PM
Subscribers
None

Details

Summary

Ref T10262. Currently, we always render a tag like this when you {F123} an image in remarkup:

<img src="/xform/preview/abcdef/" />

This either generates the preview or redirects to an existing preview. This is a good behavior in general, because the preview may take a while to generate and we don't want to wait for it to generate on the server side.

However, this flickers a lot in Safari. We might be able to cache this, but we really shouldn't, since the preview URI isn't a legitimately stable/permanent one.

Instead, do a (cheap) server-side check to see if the preview already exists. If it does, return a direct URI. This gives us a stable thumbnail in Safari.

Test Plan
  • Dragged a dog picture into comment box.
  • Typed text.
  • Thing didn't flicker like crazy all the time in Safari.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Reduce thumbnail flickering in comment previews.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
src/aphront/response/AphrontResponse.php
195–206

This isn't directly related, just cleaning up the caching headers a little and making them more explicit.

chad edited edge metadata.
This revision is now accepted and ready to land.Apr 6 2016, 10:49 PM
This revision was automatically updated to reflect the committed changes.