Skip to content

Commit 4085575

Browse files
committed
Fix remaining code review issues
- Use addActionWarning() in dialog open callback to remove duplication - Fix HTML entity display in breadcrumb and share link Literal ampersands (foo&bar) were displayed as foo&bar - Add global escapeHtml() utility function for consistent HTML escaping
1 parent ff6f2ea commit 4085575

2 files changed

Lines changed: 34 additions & 17 deletions

File tree

README.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,25 @@ We recommend using VS Code with the following plugins:
1919
* Install [DavidAnson.vscode-markdownlint](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) and when reasonable, implement its recommendations for markdown code.
2020
* Install [foxundermoon.shell-format](https://marketplace.visualstudio.com/items?itemName=foxundermoon.shell-format) and [timonwong.shellcheck](https://marketplace.visualstudio.com/items?itemName=timonwong.shellcheck) and when reasonable, implement their recommendations when making changes to bash scripts.
2121
* Install [esbenp.prettier-vscode](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode) and when reasonable, use it for formatting files.
22+
23+
## Known Issues / Future Ideas
24+
25+
### File Manager - Accurate Total Size Calculation
26+
27+
**Current Implementation:**
28+
The file manager calculates total transfer size by averaging multiple rsync progress measurements. Since rsync truncates percentages (47.9% shows as 47%), this introduces ~2% error per measurement point. We average 5 samples at different progress levels to improve accuracy and show an approximate total (prefixed with `~`).
29+
30+
**Proposed Improvement:**
31+
After collecting initial samples, run `timeout 60 du -sb $source` in the background to get exact byte count:
32+
- Parse source paths from `/var/tmp/file.manager.active` JSON
33+
- Start `du` asynchronously once estimation completes
34+
- Replace estimated size with exact size when `du` finishes
35+
- Use timeout to prevent hangs on very large directories
36+
37+
**Challenge:**
38+
For move operations using rsync copy-delete, source files get deleted during transfer. Running `du` after the operation starts would return incomplete results. Would need to:
39+
- Start `du` before rsync starts (adds delay)
40+
- Or accept that move operations keep using estimation
41+
- Or detect rsync-rename moves (same filesystem) where source remains until completion
42+
43+
**Decision:** Document for future consideration. Current estimation method is reasonable trade-off between accuracy and complexity.

emhttp/plugins/dynamix/Browse.page

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ function autoscale(value) {
5959
return ((Math.round(scale*data)/scale)+' '+unit[base]).replace('.','<?=$display['number'][0]?>')+'/s';
6060
}
6161

62+
// HTML-escape function to preserve literal &amp; and other entities
63+
function escapeHtml(text) {
64+
var div = document.createElement('div');
65+
div.textContent = text;
66+
return div.innerHTML;
67+
}
68+
6269
// Add warning to dialog buttonpane based on action type
6370
function addActionWarning(action, isBulk) {
6471
var warningTexts = isBulk ? {
@@ -1063,21 +1070,7 @@ function doActions(action, title) {
10631070
$('.ui-dfm').off('mousedown.dfmFileTree');
10641071
},
10651072
open: function() {
1066-
// Add warning to buttonpane for all relevant actions (bulk operations)
1067-
var warningTexts = {
1068-
0: "<?=_("This creates a folder at the current level")?>",
1069-
1: "<?=_("This deletes all selected sources")?>",
1070-
2: "<?=_("This renames the selected source")?>",
1071-
3: "<?=_("This copies all the selected sources")?>",
1072-
4: "<?=_("This moves all the selected sources")?>",
1073-
11: "<?=_("This changes the owner of the source recursively")?>",
1074-
12: "<?=_("This changes the permission of the source recursively")?>"
1075-
};
1076-
var warningText = warningTexts[action];
1077-
if (warningText) {
1078-
var $warning = $('<div class="dfm-warning">').html('<i class="fa fa-warning dfm"></i> ' + warningText);
1079-
$('.ui-dfm .ui-dialog-buttonset').prepend($warning);
1080-
}
1073+
addActionWarning(action, true);
10811074
},
10821075
buttons: {
10831076
"_(Start)_": function(){
@@ -1356,10 +1349,11 @@ function loadList() {
13561349

13571350
function xlink(link) {
13581351
var path = decodeURIComponent(link).trim();
1352+
var escapedPath = escapeHtml(path);
13591353

13601354
// Always show dialog with selectable textarea (better mobile support)
13611355
var inputId = 'dfm_path_input_' + Date.now();
1362-
var inputHtml = '<textarea id="' + inputId + '" class="dfm-path-textarea text-center font-mono" readonly>' + path + '</textarea>';
1356+
var inputHtml = '<textarea id="' + inputId + '" class="dfm-path-textarea text-center font-mono" readonly>' + escapedPath + '</textarea>';
13631357

13641358
swal({
13651359
title: '',
@@ -1418,7 +1412,8 @@ $(function(){
14181412
if (dirs.length > 1) {
14191413
for (var n=1; n < dirs.length; n++) {
14201414
var subdir = dirs.slice(1,n+1);
1421-
url.push('<a class="none" href="/<?=$path?>?dir=/'+encodeURIComponent(subdir.join('/'))+'" oncontextmenu="xlink(\'/' + encodeURIComponent(subdir.join('/')) + '\');return false">'+(n == 1 ? '<i class="fa fa-home"></i>' : dirs[n])+'</a>');
1415+
var displayText = n == 1 ? '<i class="fa fa-home"></i>' : escapeHtml(dirs[n]);
1416+
url.push('<a class="none" href="/<?=$path?>?dir=/'+encodeURIComponent(subdir.join('/'))+'" oncontextmenu="xlink(\'/' + encodeURIComponent(subdir.join('/')) + '\');return false">'+displayText+'</a>');
14221417
}
14231418
} else {
14241419
url.push('<i class="fa fa-home red-text"></i>');

0 commit comments

Comments
 (0)