Page MenuHomePhabricator

Macro images don't show up on html emails if your instance requires logins
Closed, ResolvedPublic

Description

If your Phabircator instance requires a login, macro images are not viewable without a login.
This causes the following issue: Gmail tries to cache the macro image for you, fails (since it doesn't have access to your phabricator instance), and you get this:

pasted_file (396×696 px, 11 KB)

Event Timeline

talshiri raised the priority of this task from to Needs Triage.
talshiri updated the task description. (Show Details)
talshiri added projects: Mail, Phabricator.
talshiri added subscribers: talshiri, lpriestley, epriestley.

Some possible approaches here:

  1. Never render <img /> tags, and render a text alternative instead (simpler).
  2. Render <img /> tags if PhabricatorEnv::getEnvConfig('policy.allow-public') is true, and a text alternative if not (more complex, but maybe slightly better).
    • This will break old email a little bit if an install goes un-public.
    • Allowing public users isn't exactly the same as allowing Google to pull images off the server, so this might still not work for some installs.
    • These installs are rare, and special casing for them may not be worthwhile.
  3. Always provide public/CDN access to macros/memes (most complex, but I think I'd expect this to be the behavior today, so the lack of it might be a bug).
    • This will never work for many installs, which are VPN'd or otherwise not accessible on the public internet.

I think (1) or (2) are most reasonable; (3) is probably not worth pursuing because of VPN installs.

As a text alternative, we could either:

  • Render just the text (shipitquick) or some embellished version of the text (<shipitquick>); or
  • allow users to provide an alternate text description of a macro (<shipitquick: a titanic cruise vessel has run aground; woe!, with text above "hmm okay" and text below "what could go wrong">); or
  • use PhabricatorBotMacroHandler::rasterize() to convert macros into ASCII art (maybe scaling down the font size to render in more detail), then draw the text on top of them.

Rasterizing macros is hilarious, but also the most work, and will probably be too intense for many users, requiring us to add some kind of option to disable it. We also may not be able to rasterize all images, and can't accurately rasterize animated GIFs. Alternate text is somewhat hilarious, but requires some work. Rendering simple text is straightforward, but is not hilarious.

I strongly favor rasterizing eventually and think it's so funny I'm even OK with adding an option for it, but a reasonable v1 would be to just render the macro plaintext so these mails always work. That's a ~3 line change vs like 1K+ for rasterizing.

chad triaged this task as Normal priority.Dec 2 2014, 4:26 PM

What about inlining the images with data:base64 ?
Sure it will make emails with images bigger, but it will always work and most emails don't have macros.

A couple of sources I've seen say Gmail strips data URIs. I haven't actually tested them, but that's why I'm ruling them out. Maybe worth actually testing.

Sadface.

It would be nice (and maybe pretty complex) for (3) to work if you happen to use S3 (which would work if you instance is behind VPN), although right now the Phabricator guide does not recommend using it.

eadler added a project: Restricted Project.Feb 17 2016, 8:02 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.
eadler removed a project: Restricted Project.Mar 24 2016, 6:11 PM