commit af19928d1565e748b35099ab8ee047de097be57b Author: Andre Klapper Date: Tue Apr 2 21:24:58 2024 +0200 Replace expensive $ fragment links with standard # HTML anchors Create an HTML anchor fragment via `'id' => $line_number` that we can link to, and replace the $ with # in the URIs. The `$1-10` range syntax will still work (and might be posted across places in Phorge) but webcrawlers on Diffusion source code pages and in Pastes will only find standard # anchor links for the very same pa ge not to follow. Which is our goal per https://we.phorge.it/T15670 when it comes to reducing distractive webcrawler behavior. For general understanding: * src/view/layout/PhabricatorSourceCodeView.php calls `Javelin::initBehavior('phabricator-line-linker')`. * webroot/rsrc/js/core/behavior-line-linker.js calls `JX.DOM.alterClass(cursor, 'phabricator-source-highlight', true)` to set the background color of a highlighted line via CSS. * webroot/rsrc/js/core/behavior-line-linker.js calls `JX.DOM.scrollToPosition(0, JX.$V(anchor).y - 60);` to make the browser jump to the highlighted range, but again we cannot enforce highlighting of a line "the other way round" here as the scroll position update is done locally by the browser only (and the browser jumps to the anchor line as the very first line, instead of 60px above as the server-side JS does her e) * We cannot highlight our anchor line in `AphrontRequest.php`'s `getURILineRange` as the fragment identifier is never sent to our server but only locally handled in the browser. So we lose highlighting with the anchor approach. Test Plan: * Go to http://phorge.localhost/P1, click on line number 47, see that highlighting still works and URL in the address bar becomes http://phorge.localhost/P1#47 without a page reload * Go to http://phorge.localhost/P1#33 and see that scrolling works as expected * Try http://phorge.localhost/P1$38-40 syntax and see that highlighting still works as expected * Do the same test in a source code file rendered in Diffusion diff --git a/src/view/layout/PhabricatorSourceCodeView.php b/src/view/layout/PhabricatorSourceCodeView.php index ae20fe2501..fd4db19176 100644 --- a/src/view/layout/PhabricatorSourceCodeView.php +++ b/src/view/layout/PhabricatorSourceCodeView.php @@ -132,7 +132,7 @@ final class PhabricatorSourceCodeView extends AphrontView { if ($this->canClickHighlight) { if ($base_uri) { - $line_href = $base_uri.'$'.$line_number; + $line_href = $base_uri.'#'.$line_number; } else { $line_href = null; } @@ -142,6 +142,7 @@ final class PhabricatorSourceCodeView extends AphrontView { array( 'href' => $line_href, 'data-n' => $line_number, + 'id' => $line_number, )); } else { $tag_number = phutil_tag( diff --git a/webroot/rsrc/js/core/behavior-line-linker.js b/webroot/rsrc/js/core/behavior-line-linker.js index fd8510646e..611a3311c2 100644 --- a/webroot/rsrc/js/core/behavior-line-linker.js +++ b/webroot/rsrc/js/core/behavior-line-linker.js @@ -164,7 +164,7 @@ JX.behavior('phabricator-line-linker', function() { uri = JX.$U(uri); path = uri.getPath(); - path = path + '$' + lines; + path = path + '#' + lines; uri = uri.setPath(path).toString(); JX.History.replace(uri);