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
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
F13190661: D21709.id.diff
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
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
Branch
phriction2
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 25519
Build 35281: Run Core Tests
Build 35280: arc lint + arc unit

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