Page MenuHomePhabricator

Pass a real context object to Phriction previews, fixing mentions
ClosedPublic

Authored by epriestley on Jul 26 2021, 6:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 4:47 AM
Unknown Object (File)
Fri, Nov 15, 9:42 AM
Unknown Object (File)
Wed, Nov 13, 3:08 AM
Unknown Object (File)
Mon, Nov 11, 5:42 AM
Unknown Object (File)
Fri, Nov 8, 9:21 PM
Unknown Object (File)
Thu, Nov 7, 10:56 PM
Unknown Object (File)
Thu, Nov 7, 12:08 PM
Unknown Object (File)
Sat, Nov 2, 7:09 AM
Subscribers
None

Details

Summary

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

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/applications/phriction/markup/PhrictionRemarkupRule.php
284

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 @gmail.com, but do not have a user named gmail.com 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 (@gmail.com), not fail loudly ("Error: unable to render remarkup because it contains an invalid mention, no such user 'gmail.com'!"), 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