Page MenuHomePhabricator

Reduce thumbnail flickering in comment previews
ClosedPublic

Authored by epriestley on Apr 6 2016, 10:32 PM.

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley updated this revision to Diff 37704.Apr 6 2016, 10:32 PM
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.
epriestley added inline comments.Apr 6 2016, 10:33 PM
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 accepted this revision.Apr 6 2016, 10:49 PM
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.