Page MenuHomePhabricator

Fix possible recursive embeds in Dashboard text panels
ClosedPublic

Authored by epriestley on Aug 27 2015, 12:43 AM.

Details

Summary

We currently detect tab panels embedding themselves, but do not detect text panels embedding themselves with {Wxx}.

Detect these self-embedding panels.

I had to add a bit of a hack to pass the parent panel PHIDs to the rule. Generally, I got the Markup API kind of wrong and want to update it, I'll file a followup with details about how I'd like to move forward.

Test Plan

Created a text panel embedding itself, a tab panel embedding a text panel embedding itself, a tab panel embedding a text panel embedding the tab panel, etc.

Rendered all panels standalone and as {Wxx} from a different context.

Screen Shot 2015-08-26 at 5.39.44 PM.png (231×1 px, 32 KB)

Screen Shot 2015-08-26 at 5.40.50 PM.png (239×1 px, 35 KB)

Screen Shot 2015-08-26 at 5.39.59 PM.png (315×1 px, 46 KB)

Screen Shot 2015-08-26 at 5.41.01 PM.png (343×1 px, 47 KB)

Screen Shot 2015-08-26 at 5.40.39 PM.png (242×1 px, 35 KB)

Diff Detail

Repository
rP Phabricator
Branch
tpanel1
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/infrastructure/markup/PhabricatorMarkupEngine.php:319XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 7819
Build 8670: [Placeholder Plan] Wait for 30 Seconds
Build 8669: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Fix possible recursive embeds in Dashboard text panels.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added reviewers: chad, jbeta.
chad edited edge metadata.
This revision is now accepted and ready to land.Aug 27 2015, 12:45 AM
jbeta edited edge metadata.

Applied this diff on my test install, panels confirmed unhaunted.

T9273 is my note-to-myself about fixing this API, which has grown over time to have about 4 distinct entry points, none of which map well to actual usage or really need to exist.

This revision was automatically updated to reflect the committed changes.