1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-21 22:32:41 +01:00

Correct minor "jump to symbol" behavior in Differential

Summary:
Ref T13644. Ref T13638.

  - Double-encode the symbol that is used as a path component, similar to Diffusion.
  - Fix an outdated reference to ".path", which provided context for symbol lookup.
  - Prevent command-clicking headers from looking up the path as a symbol.

Test Plan:
  - Command-clicked headers, no longer got a symbol.
  - Command-clicked stuff with "/", saw it double-encoded and decoded properly.
  - Command-clicked normal symbols, saw "path" populate correctly.

Maniphest Tasks: T13644, T13638

Differential Revision: https://secure.phabricator.com/D21641
This commit is contained in:
epriestley 2021-03-17 15:27:58 -07:00
parent d6ed9392d4
commit db9191f9a8
5 changed files with 46 additions and 26 deletions

View file

@ -13,7 +13,7 @@ return array(
'core.pkg.js' => 'ab3502fe',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => 'ffb69e3d',
'differential.pkg.js' => '5080baf4',
'differential.pkg.js' => '5986f349',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => '78c9885d',
'maniphest.pkg.css' => '35995d6d',
@ -437,7 +437,7 @@ return array(
'rsrc/js/application/releeph/releeph-preview-branch.js' => '75184d68',
'rsrc/js/application/releeph/releeph-request-state-change.js' => '9f081f05',
'rsrc/js/application/releeph/releeph-request-typeahead.js' => 'aa3a100c',
'rsrc/js/application/repository/repository-crossreference.js' => '6337cf26',
'rsrc/js/application/repository/repository-crossreference.js' => '44d48cd1',
'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e5bdb730',
'rsrc/js/application/search/behavior-reorder-queries.js' => 'b86f297f',
'rsrc/js/application/transactions/behavior-comment-actions.js' => '4dffaeb2',
@ -693,7 +693,7 @@ return array(
'javelin-behavior-reorder-applications' => 'aa371860',
'javelin-behavior-reorder-columns' => '8ac32fd9',
'javelin-behavior-reorder-profile-menu-items' => 'e5bdb730',
'javelin-behavior-repository-crossreference' => '6337cf26',
'javelin-behavior-repository-crossreference' => '44d48cd1',
'javelin-behavior-scrollbar' => '92388bae',
'javelin-behavior-search-reorder-queries' => 'b86f297f',
'javelin-behavior-select-content' => 'e8240b50',
@ -1309,6 +1309,12 @@ return array(
'43bc9360' => array(
'javelin-install',
),
'44d48cd1' => array(
'javelin-behavior',
'javelin-dom',
'javelin-stratcom',
'javelin-uri',
),
'457f4d16' => array(
'javelin-behavior',
'javelin-stratcom',
@ -1536,12 +1542,6 @@ return array(
'javelin-request',
'javelin-uri',
),
'6337cf26' => array(
'javelin-behavior',
'javelin-dom',
'javelin-stratcom',
'javelin-uri',
),
'65bb0011' => array(
'javelin-behavior',
'javelin-dom',

View file

@ -246,6 +246,7 @@ final class DifferentialChangesetDetailView extends AphrontView {
'displayPath' => hsprintf('%s', $display_parts),
'icon' => $display_icon,
'pathParts' => $path_parts,
'symbolPath' => $display_filename,
'pathIconIcon' => $changeset->getPathIconIcon(),
'pathIconColor' => $changeset->getPathIconColor(),

View file

@ -4,7 +4,10 @@ final class DiffusionSymbolController extends DiffusionController {
public function handleRequest(AphrontRequest $request) {
$viewer = $this->getViewer();
// See T13638 for discussion of escaping.
$name = $request->getURIData('name');
$name = phutil_unescape_uri_path_component($name);
$query = id(new DiffusionSymbolQuery())
->setViewer($viewer)

View file

@ -66,12 +66,12 @@ final class DiffusionDatasourceEngineExtension
$parts = null;
if (preg_match('/(.*)(?:\\.|::|->)(.*)/', $symbol, $parts)) {
return urisprintf(
'/diffusion/symbol/%s/?jump=true&context=%s',
'/diffusion/symbol/%p/?jump=true&context=%s',
$parts[2],
$parts[1]);
} else {
return urisprintf(
'/diffusion/symbol/%s/?jump=true',
'/diffusion/symbol/%p/?jump=true',
$symbol);
}
}

View file

@ -66,13 +66,8 @@ JX.behavior('repository-crossreference', function(config, statics) {
var target = e.getTarget();
try {
// If we're in an inline comment, don't link symbols.
if (JX.DOM.findAbove(target, 'div', 'differential-inline-comment')) {
return;
}
} catch (ex) {
// Continue if we're not inside an inline comment.
if (!canLinkNode(target)) {
return;
}
// If only part of the symbol was edited, the symbol name itself will
@ -97,6 +92,29 @@ JX.behavior('repository-crossreference', function(config, statics) {
}
});
}
function canLinkNode(node) {
try {
// If we're in an inline comment, don't link symbols.
if (JX.DOM.findAbove(node, 'div', 'differential-inline-comment')) {
return false;
}
} catch (ex) {
// Continue if we're not inside an inline comment.
}
// See T13644. Don't open symbols if we're inside a changeset header.
try {
if (JX.DOM.findAbove(node, 'h1')) {
return false;
}
} catch (ex) {
// Continue if not inside a header.
}
return true;
}
function unhighlight() {
highlighted && JX.DOM.alterClass(highlighted, classHighlight, false);
highlighted = null;
@ -159,6 +177,9 @@ JX.behavior('repository-crossreference', function(config, statics) {
uri_symbol = uri_symbol.trim();
// See T13437. Symbols like "#define" need to be encoded.
// See T13644. Symbols like "a/b" must be double-encoded to survive
// one layer of decoding by the webserver.
uri_symbol = encodeURIComponent(uri_symbol);
uri_symbol = encodeURIComponent(uri_symbol);
var uri = JX.$U('/diffusion/symbol/' + uri_symbol + '/');
@ -223,7 +244,7 @@ JX.behavior('repository-crossreference', function(config, statics) {
var changeset;
try {
changeset = JX.DOM.findAbove(target, 'div', 'differential-changeset');
return JX.Stratcom.getData(changeset).path;
return JX.Stratcom.getData(changeset).symbolPath || null;
} catch (ex) {
// Ignore.
}
@ -315,13 +336,8 @@ JX.behavior('repository-crossreference', function(config, statics) {
var target = e.getTarget();
try {
// If we're in an inline comment, don't link symbols.
if (JX.DOM.findAbove(target, 'div', 'differential-inline-comment')) {
return;
}
} catch (ex) {
// Continue if we're not inside an inline comment.
if (!canLinkNode(target)) {
return;
}
// If only part of the symbol was edited, the symbol name itself will