Page MenuHomePhorge

D25118.1734619316.diff
No OneTemporary

D25118.1734619316.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -5789,6 +5789,8 @@
'PhutilTranslatedHTMLTestCase' => 'infrastructure/markup/__tests__/PhutilTranslatedHTMLTestCase.php',
'PhutilTwitchAuthAdapter' => 'applications/auth/adapter/PhutilTwitchAuthAdapter.php',
'PhutilTwitterAuthAdapter' => 'applications/auth/adapter/PhutilTwitterAuthAdapter.php',
+ 'PhutilURIHelper' => 'infrastructure/parser/PhutilURIHelper.php',
+ 'PhutilURIHelperTestCase' => 'infrastructure/parser/__tests__/PhutilURIHelperTestCase.php',
'PhutilWordPressAuthAdapter' => 'applications/auth/adapter/PhutilWordPressAuthAdapter.php',
'PhutilXHPASTSyntaxHighlighter' => 'infrastructure/markup/syntax/highlighter/PhutilXHPASTSyntaxHighlighter.php',
'PhutilXHPASTSyntaxHighlighterFuture' => 'infrastructure/markup/syntax/highlighter/xhpast/PhutilXHPASTSyntaxHighlighterFuture.php',
@@ -12675,6 +12677,8 @@
'PhutilTranslatedHTMLTestCase' => 'PhutilTestCase',
'PhutilTwitchAuthAdapter' => 'PhutilOAuthAuthAdapter',
'PhutilTwitterAuthAdapter' => 'PhutilOAuth1AuthAdapter',
+ 'PhutilURIHelper' => 'Phobject',
+ 'PhutilURIHelperTestCase' => 'PhabricatorTestCase',
'PhutilWordPressAuthAdapter' => 'PhutilOAuthAuthAdapter',
'PhutilXHPASTSyntaxHighlighter' => 'Phobject',
'PhutilXHPASTSyntaxHighlighterFuture' => 'FutureProxy',
diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php
--- a/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php
+++ b/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php
@@ -44,16 +44,16 @@
protected function renderHyperlink($link, $name) {
$engine = $this->getEngine();
- $is_anchor = false;
- if (strncmp($link, '/', 1) == 0) {
+ $uri = new PhutilURIHelper($link);
+ $is_anchor = $uri->isAnchor();
+ $starts_with_slash = $uri->isStartingWithSlash();
+ if ($starts_with_slash) {
$base = phutil_string_cast($engine->getConfig('uri.base'));
$base = rtrim($base, '/');
$link = $base.$link;
- } else if (strncmp($link, '#', 1) == 0) {
+ } else if ($is_anchor) {
$here = $engine->getConfig('uri.here');
$link = $here.$link;
-
- $is_anchor = true;
}
if ($engine->isTextMode()) {
@@ -76,7 +76,13 @@
return $name;
}
- $same_window = $engine->getConfig('uri.same-window', false);
+ // Check if this link points to Phorge itself. Micro-optimized.
+ $is_self = $is_anchor || $starts_with_slash || $uri->isSelf();
+
+ // For historical reasons, links opened in a different tab
+ // for most links as default.
+ // Now internal resources keep internal link, as default.
+ $same_window = $engine->getConfig('uri.same-window', $is_self);
if ($same_window) {
$target = null;
} else {
@@ -92,7 +98,7 @@
'a',
array(
'href' => $link,
- 'class' => 'remarkup-link',
+ 'class' => $this->getRemarkupLinkClass($is_self),
'target' => $target,
'rel' => 'noreferrer',
),
diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupHyperlinkRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupHyperlinkRule.php
--- a/src/infrastructure/markup/markuprule/PhutilRemarkupHyperlinkRule.php
+++ b/src/infrastructure/markup/markuprule/PhutilRemarkupHyperlinkRule.php
@@ -116,7 +116,9 @@
$engine = $this->getEngine();
- $same_window = $engine->getConfig('uri.same-window', false);
+ $uri = new PhutilURIHelper($link);
+ $is_self = $uri->isSelf();
+ $same_window = $engine->getConfig('uri.same-window', $is_self);
if ($same_window) {
$target = null;
} else {
@@ -127,7 +129,7 @@
'a',
array(
'href' => $link,
- 'class' => 'remarkup-link',
+ 'class' => $this->getRemarkupLinkClass($is_self),
'target' => $target,
'rel' => 'noreferrer',
),
diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php
--- a/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php
+++ b/src/infrastructure/markup/markuprule/PhutilRemarkupRule.php
@@ -106,4 +106,20 @@
return (strpos($text, PhutilRemarkupBlockStorage::MAGIC_BYTE) === false);
}
+ /**
+ * Get the CSS class="" attribute for a Remarkup link.
+ * It's just "remarkup-link" for all cases, plus the possibility for
+ * designers to style external links differently.
+ * @param boolean $is_internal Whenever the link was internal or not.
+ * @return string
+ */
+ protected function getRemarkupLinkClass($is_internal) {
+ // Allow developers to style esternal links differently
+ $classes = array('remarkup-link');
+ if (!$is_internal) {
+ $classes[] = 'remarkup-link-ext';
+ }
+ return implode(' ', $classes);
+ }
+
}
diff --git a/src/infrastructure/parser/PhutilURIHelper.php b/src/infrastructure/parser/PhutilURIHelper.php
new file mode 100644
--- /dev/null
+++ b/src/infrastructure/parser/PhutilURIHelper.php
@@ -0,0 +1,78 @@
+<?php
+
+/**
+ * A simple wrapper for PhutilURI, to be aware of the
+ * relative/absolute context, and other minor things.
+ */
+final class PhutilURIHelper extends Phobject {
+
+ /**
+ * String version of your original URI.
+ * @var string
+ */
+ private $uriStr;
+
+ /**
+ * Structured version of your URI.
+ * @var PhutilURI
+ */
+ private $phutilUri;
+
+ /**
+ * @param string|PhutilURI
+ */
+ public function __construct($uri) {
+
+ // Keep the original string for basic checks.
+ $this->uriStr = phutil_string_cast($uri);
+
+ // A PhutilURI may be useful. If available, import that as-is.
+ // Note that the constructor PhutilURI(string) is a bit expensive.
+ if ($uri instanceof PhutilURI) {
+ $this->phutilUri = $uri;
+ }
+ }
+
+ /**
+ * Check if the URI points to Phorge itself.
+ * @return bool
+ */
+ public function isSelf() {
+ // The backend prefers a PhutilURI object, if available.
+ $uri = $this->phutilUri ? $this->phutilUri : $this->uriStr;
+ return PhabricatorEnv::isSelfURI($uri);
+ }
+
+ /**
+ * Check whenever an URI is just a simple fragment without path and protocol.
+ * @return bool
+ */
+ public function isAnchor() {
+ return $this->isStartingWithChar('#');
+ }
+
+ /**
+ * Check whenever an URI starts with a slash (no protocol, etc.)
+ * @return bool
+ */
+ public function isStartingWithSlash() {
+ return $this->isStartingWithChar('/');
+ }
+
+ /**
+ * A sane default.
+ */
+ public function __toString() {
+ return $this->uriStr;
+ }
+
+ /**
+ * Check whenever the URI starts with the provided character.
+ * @param string $char String that MUST have length of 1.
+ * @return boolean
+ */
+ private function isStartingWithChar($char) {
+ return strncmp($this->uriStr, $char, 1) === 0;
+ }
+
+}
diff --git a/src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php b/src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php
@@ -0,0 +1,63 @@
+<?php
+
+final class PhutilURIHelperTestCase extends PhabricatorTestCase {
+
+ public function testPhutilURIHelper() {
+
+ // Every row is a test. Every column is:
+ // - 0: name of the test
+ // - 1: test input value
+ // - 2: is the URI pointing to Phorge itself?
+ // - 3: is the URI an anchor? (no domain, no protocol)
+ // - 4: is the URI starting with a slash? (no domain, no protocol)
+ $tests = array(
+ array('internal anchor', '#asd', true, true, false),
+ array('internal relative dir', '/foo/', true, false, true),
+ array('internal relative dir also', 'foo/', true, false, false),
+ array('internal root dir', '/', true, false, true),
+ array('internal root dir', './', true, false, false),
+ array('internal root dir', '../', true, false, false),
+ array('internal root dir', '/#asd', true, false, true),
+ array('external', 'https://gnu.org/', false, false, false),
+ array('external anchor', 'https://gnu.org/#asd', false, false, false),
+ );
+
+ // Add additional self-tests if base URI is available.
+ $base = PhabricatorEnv::getEnvConfigIfExists('phabricator.base-uri');
+ if ($base) {
+ $domain = id(new PhutilURI($base))->getDomain();
+ $tests[] = array('base uri', $base, true, false, false);
+ $tests[] = array('base uri anchor', "{$base}#asd", true, false, false);
+ }
+
+ foreach ($tests as $test) {
+ $name = $test[0];
+ $uri = $test[1];
+ $is_self = $test[2];
+ $is_anchor = $test[3];
+ $is_slash = $test[4];
+
+ // Test input variants for the constructor of PhutilURIHelper.
+ $uri_variants = array(
+ $uri,
+ new PhutilURI($uri),
+ );
+ foreach ($uri_variants as $variant_uri) {
+
+ $test_name = pht("test %s value '%s' (from '%s' type %s)",
+ $name, $variant_uri, $uri, phutil_describe_type($variant_uri));
+
+ $uri = new PhutilURIHelper($variant_uri);
+
+ $this->assertEqual($is_self, $uri->isSelf(),
+ pht('%s - points to myself', $test_name));
+
+ $this->assertEqual($is_anchor, $uri->isAnchor(),
+ pht('%s - is just an anchor', $test_name));
+
+ $this->assertEqual($is_slash, $uri->isStartingWithSlash(),
+ pht('%s - is starting with slash', $test_name));
+ }
+ }
+ }
+}

File Metadata

Mime Type
text/plain
Expires
Thu, Dec 19, 14:41 (18 h, 19 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1015025
Default Alt Text
D25118.1734619316.diff (9 KB)

Event Timeline