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
F15509930: D21709.id51716.diff
Wed, Apr 16, 5:35 PM
F15470886: D21709.diff
Sat, Apr 5, 1:55 AM
F15463080: D21709.diff
Tue, Apr 1, 8:07 PM
F15455949: D21709.diff
Sun, Mar 30, 5:54 AM
F15438963: D21709.id51732.diff
Wed, Mar 26, 5:01 AM
F15438677: D21709.id51716.diff
Wed, Mar 26, 2:36 AM
F15431786: D21709.id.diff
Mar 24 2025, 2:31 PM
F15424043: D21709.id51716.diff
Mar 22 2025, 7:59 PM
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