Page MenuHomePhabricator

Support macros in Phame
Open, WishlistPublic

Description

Reproduction steps:

  • Create a post on Phame
  • Insert a macro using the Meme button, and include some text.
  • Check Blog Post (Preview) below the writing area - the Macros all display as normal and expected.
  • Click Create New Post or Save Changes as appropriate.
  • View the post through Phame or in Live form.

Expected result:

  • The macros should display on the Phame post, when viewed via Phame or "Live".

Actual result:

  • The macros do NOT display; you only see the Remarkup code for the macros.

Version Infodump
phabricator 9124bb416241bb117d50ced260e3b23e830882dd (Sat, Mar 4) (branched from 8e26916f7faf4a3348c22e528824f0088e7a6275 on origin) arcanist 822bc53ca306e06314560d8a76f68771d732e8e0 (Fri, Feb 24) (branched from 3b6b523c2b236e3724a1e115f126cb6fd05fa128 on origin) phutil 796cb1c2ee274397a8a7bc6c10566fd751619d6d (Sat, Mar 4) (branched from 13a200ca7621ab2b48a0c395f52f8c4411bbc686 on origin)
NOTE: Server automatically pulls in the latest from the stable branch every day and restarts appropriately.

Event Timeline

chad triaged this task as Low priority.Mar 9 2017, 2:47 AM

Macros are disabled in Phame posts, via markup engine. This originated in D2202.

It may have been copy/pasted from Feed (which disables macros for reasons similar to T11845) or might have just been a product decision aimed at aligning Phame more with serious corporate blog use cases.

I think there's no reason to prevent use of macros in Phame posts in the modern product, which targets both internal and external use cases. It's also not obvious why macros wouldn't work in Phame, while it's more obvious why we no longer allow autoplay looped audio in Feed.

@chad, any thoughts here or better recollection about why we disabled them? If you don't have any better reasons than I do, I believe fixing this is just an issue of removing this line:

I can fix if it's one line, I just assumed it was something super hard with external urls or something.

I guess we actually have to test this use case:

  • No security.alternate-file-domain configured.
  • Blog on an external domain.

That might incorrectly link to /file/data/... which won't exist. If so, the Macro remarkup rule needs to respect the uri.full option.

That is, the problem would be:

  • You visit blog.install.com.
  • It generates links to /file/data/..., which won't exist (that path isn't served by blog.install.com).
  • It should be generating links to https://real-phabricator.install.com/file/data/... instead.
epriestley renamed this task from Macro not displaying in Phame post to Support macros in Phame.Mar 11 2017, 6:56 PM
epriestley lowered the priority of this task from Low to Wishlist.

Happy to accept a patch for this (deleting that one line of code) if someone wants to claim that they tested the scenario above and it worked correctly.

@epriestley, I wouldn't mind testing that scenario out this weekend (I'll have to change a couple of domain name settings to create the scenario, but that's easily undone).

In the meantime, you should be aware that I changed that one line from false to true in my copy of Phabricator, but Macros still aren't rendering in Phame. (I also tried removing that line, no dice.)

Remarkup is cached, so you'll need to run bin/cache purge --purge-remarkup (or make an edit, or create a new post) to purge the cache.

Ah! That fixed it, thanks @epriestley. I'll let you know once I test out that configuration.

@epriestley, I tested out the scenario with the change applied.

  • Turned off Alternate File Domain.
  • Set custom domain name for blog.
  • Set Phame and blog visibility to Public.
  • Created post in blog with macro.
  • Published post.
  • Viewed post "live" - the post displays, but the image URL is incorrect, pointing to the blog URL (and therefore giving a 404).

This is the case, regardless of whether the parent URL is set on the blog.