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 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.