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)
Wed, Dec 4, 7:33 AM
Unknown Object (File)
Tue, Dec 3, 5:34 AM
Unknown Object (File)
Mon, Dec 2, 3:00 PM
Unknown Object (File)
Wed, Nov 27, 7:17 AM
Unknown Object (File)
Wed, Nov 27, 3:15 AM
Unknown Object (File)
Mon, Nov 25, 6:10 PM
Unknown Object (File)
Sat, Nov 23, 8:47 AM
Unknown Object (File)
Nov 22 2024, 7:02 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