Specialize rendering of self-URIs in the form "/X123"
ClosedPublic
Actions

Authored by epriestley on May 9 2019, 8:34 PM.

Details

Reviewers

amckinley
joshuaspence

Maniphest Tasks

T5378: Phabricator full task URLs for that instance do not provide hover/popup text (like T1234 does)

Summary

Depends on D20510. Ref T5378. When remarkup includes a hyperlink to the current install in the form "/X123" (which is common), load the corresponding object and specialize the rendering.

This doesn't cover everything (notably, no handling for Diffusion paths yet), but does cover a lot of the most common cases.

The "uri" form preserves the URI as written, but adds an icon, tag, and hovercard.

The "{uri}" form is more similar to {T123} and shows the object name.

Test Plan

Screen Shot 2019-05-09 at 1.32.02 PM.png (540×615 px, 51 KB)

Diff Detail

Repository

rP Phabricator

Branch

sref4

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 22778
Build 31243: Run Core Tests
Build 31242: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	199 ms	PhabricatorLibraryTestCase::testEverythingImplemented EXCEPTION (PhutilMissingSymbolException): Failed to load class/interface "PhabricatorSelfHyperlinkEngineExtension". The symbol map for library "phabricator" (at "/core/data/drydock/workingcopy-90/repo/phabricator/src") claims this symbol (of type "class/interface") is defined in "applications/meta/engineextension/PhabricatorSelfHyperlinkEngineExtension.php", but loading that source file did not cause the symbol to become defined.
	1,046 ms	PhabricatorLibraryTestCase::testMethodVisibility EXCEPTION (PhutilMissingSymbolException): Failed to load class or interface "PhutilRemarkupHyperlinkEngineExtension". The class or interface "PhutilRemarkupHyperlinkEngineExtension" is not defined in the library map of any loaded library.
	1 ms	AlmanacNamesTestCase::testServiceOrDeviceNames 30 assertions passed.
	0 ms	AlmanacServiceTypeTestCase::testGetAllServiceTypes 1 assertion passed.
	0 ms	AphrontHTTPSinkTestCase::testHTTPHeaderNames 2 assertions passed.
		View Full Test Results (2 Failed · 362 Passed)

Event Timeline

epriestley created this revision.May 9 2019, 8:34 PM

Slightly more complete test case:

Screen Shot 2019-05-09 at 1.34.23 PM.png (563×591 px, 54 KB)

Harbormaster returned this revision to the author for changes because remote builds failed.May 9 2019, 8:35 PM

Harbormaster failed remote builds in B22778: Diff 48905!

(Test failures are because of D20511 not being landed yet.)

epriestley mentioned this in D12692: Allow full URIs to be rendered as object references.May 9 2019, 8:41 PM

joshuaspence accepted this revision.May 14 2019, 12:36 AM

joshuaspence added a subscriber: joshuaspence.

joshuaspence added inline comments.

src/applications/meta/engineextension/PhabricatorSelfHyperlinkEngineExtension.php
30–41	A minor point, but this will match `/robots.txt`... should the regex be tightened so as to only allow monograms?
44–46	I guess it doesn't really matter since you are looking up the objects anyway.

This revision is now accepted and ready to land.May 14 2019, 12:36 AM

epriestley added inline comments.May 16 2019, 6:25 PM

src/applications/meta/engineextension/PhabricatorSelfHyperlinkEngineExtension.php
44–46	Yeah, we'll match various `x.y.com/thing` patterns, and `/D99999999999999`, but I expect these to be pretty rare and the behavior will still be correct, we'll just do a little extra work (discarding `thing` as a possible object name, or querying for `D99999999999` and failing to load it).