Page MenuHomePhorge

Documentation: Macro image rendered instead of keyword when names match
Open, Needs TriagePublic

Description

Steps to reproduce:

  1. Phorge at 1b5d2f83c3aa03dada5cbed4a18d341733348425, PHP 8.3.9 (but irrelevant).
  2. Go to http://phorge.localhost/macro/edit/
  3. Enter progress in the Name field, upload some image, then click Create New Macro
  4. Define a custom status called "progress" in http://phorge.localhost/config/edit/maniphest.statuses/ and place it after (!) the default "resolved" status, for example
	"progress": {
		"claim": false,
		"closed": false,
		"name": "In Progress",
		"name.action": "Started",
		"name.full": "Open, In Progress",
		"transaction.icon": "fa-step-forward",
		"transaction.color": "green"
	},
  1. Go to the !status section on http://phorge.localhost/applications/mailcommands/PhabricatorManiphestApplication/task/

Expected outcome:

Table shows "progress" under "Keywords".

Actual outcome:

Macro image is displayed under "Keywords":

Screenshot from 2024-07-08 17-16-12.png (974×899 px, 119 KB)

This seems to be feature fallout as macros get embedded without any further markup, and this table cell has a single word which is also the name of a macro.
Code in https://we.phorge.it/source/phorge/browse/master/src/applications/maniphest/command/ManiphestStatusEmailCommand.php$23-44 is not to blame but rather https://we.phorge.it/source/phorge/browse/master/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php
I am mumbling words not to share.

Event Timeline

LOL

these pages should have Macro disabled, probably. A user on the install can add more macros that would break the page in some other ways. Probably.

Quick notes:

  • For testing code changes, make sure to ./bin/cache purge --caches remarkup
  • rPcc44ae32c546: Remove obsolete "setDisableMacros()" on "PhabricatorRemarkupControl" once upon a time existed, could provide a base
    • But how to even realize/pass the info that we are rendering strings in a documentation context?
  • PhutilRemarkupRule has a isFlatText($text) method but no setFlatText(true) method
  • PhutilRemarkupSimpleTableBlockRule::markupText() is handling our $cell which has the meme/macro name as its only string content
  • Afterwards, PhabricatorMemeRemarkupRule:apply() is the next step being called

As temporary Phorge workaround, we may want to document these email keywords rendered as "progress". So we can just manually add backtics (?)

As temporary downstream WMF workaround, please replace this clearly-undesired "progress" meme:

https://phabricator.wikimedia.org/macro/view/60/

With this one:

image.png (361×500 px, 208 KB)

So at least it's a meme, and it fits its purpose to be a meme 👍 So it may still be confusing, but at least in a more prominent funny way, Phorge-approved.

Edited: I know that in WMF we cannot copy-paste copyrighted materials, but memes are out of copyright in Europe. Consider paying 1 billion dollars to a lawyer to keep that proposed meme there, to at least use a Phorge-approved meme.