Page MenuHomePhorge
Paste P35

Replace fragments
ActivePublic
Actions

Authored by aklapper on Apr 2 2024, 19:32.
Tags
None
Referenced Files
F2112290: Replace fragments
Apr 2 2024, 19:34
F2112289: Replace fragments
Apr 2 2024, 19:32
Subscribers
None
Tokens
"Mountain of Wealth" token, awarded by 20after4.
commit af19928d1565e748b35099ab8ee047de097be57b
Author: Andre Klapper <a9016009@gmx.de>
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 page 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 here).
* 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);
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0