Page MenuHomePhabricator

Pass a real context object to Phriction previews, fixing mentions

Authored by epriestley on Jul 26 2021, 6:32 PM.
Referenced Files
F13217746: D21709.id51732.diff
Sat, May 18, 7:53 AM
F13215102: D21709.diff
Fri, May 17, 3:00 PM
F13196786: D21709.diff
Sun, May 12, 11:34 PM
Sat, May 11, 1:01 PM
Unknown Object (File)
Tue, May 7, 2:07 AM
Unknown Object (File)
Mon, May 6, 3:36 PM
Unknown Object (File)
Fri, May 3, 7:14 AM
Unknown Object (File)
Thu, May 2, 10:15 PM



Fixes T13662. Phriction currently passes a map as a "context object", but this code is ancient and predates the modern meaning of a "context object". In modern code, context objects should be real objects.

Provide a real object as a context object. We do this by either loading the actual document or constructing a synthetic version of it.

Test Plan
  • Edited an existing document, observing the preview:
    • Used a mention rule, saw a preview.
    • Used [[ a ]] and [[ ./a ]] absolute and relative reference rules, saw accurate previews.
  • Edited a new document, observing the preview:
    • Used a mention rule, saw a preview.
    • Used absolute/relative references, saw accurate previews.
  • Grepped for other references to the removed properties (phriction.isPreview and phriction.slug), found none remaining.

Diff Detail

rP Phabricator
Lint Passed
Tests Passed
Build Status
Buildable 25519
Build 35281: Run Core Tests
Build 35280: arc lint + arc unit

Event Timeline


Should this fail loudly instead, or are the above possibilities not meant to be exhaustive?

You can render remarkup using this rule without having a meaningful relative base URI, e.g. on some other object type or via remarkup.process in the API. This isn't necessarily a meaningful operation and the result may not be useful, but remarkup generally tries not to fail loudly: if you copy/paste a block of text from somewhere that happens to have some substrings which aren't valid, the desired behavior is generally for your text to be processed in some best-guess-at-sensible way -- and usually emitted unmodified (see also discussion in D21713).

For example, if you write or paste a piece of text which includes, but do not have a user named on your install, it is not likely that you intend to mention a user. The current desired/actual behavior is to render your input literally (, not fail loudly ("Error: unable to render remarkup because it contains an invalid mention, no such user ''!"), with a hint in the preview only that the mention isn't valid.

The particular behavior of [[ ./relative/path ]] in a non-Phriction context today is questionable (you get a link to Wiki document /w/dot/relative/path/, i.e. the default slug for a Wiki page literally named ./relative/path), but this change doesn't affect it.

This revision is now accepted and ready to land.Aug 1 2021, 7:11 PM