Fix #2500: File Manager UI/UX improvements #2501
+1,056
−203
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,8 +32,8 @@ function path($dir) { | |
| } | ||
|
|
||
| function is_top($dir) { | ||
| global $fileTreeRoot; | ||
| return mb_strlen($dir) > mb_strlen($fileTreeRoot); | ||
| } | ||
|
|
||
| function no_dots($name) { | ||
|
|
@@ -45,11 +45,14 @@ function my_dir($name) { | |
| return ($rootdir === $userdir && in_array($name, $UDincluded)) ? $topdir : $rootdir; | ||
| } | ||
|
|
||
| $fileTreeRoot = path(realpath($_POST['root'])); | ||
| if (!$fileTreeRoot) exit("ERROR: Root filesystem directory not set in jqueryFileTree.php"); | ||
|
|
||
| $docroot = '/usr/local/emhttp'; | ||
| require_once "$docroot/webGui/include/Secure.php"; | ||
| $_SERVER['REQUEST_URI'] = ''; | ||
| require_once "$docroot/webGui/include/Translations.php"; | ||
| require_once "$docroot/plugins/dynamix/include/PopularDestinations.php"; | ||
|
|
||
| $mntdir = '/mnt/'; | ||
| $userdir = '/mnt/user/'; | ||
|
|
@@ -64,13 +67,51 @@ function my_dir($name) { | |
| // Included UD shares to show under '/mnt/user' | ||
| $UDincluded = ['disks','remotes']; | ||
|
|
||
| $showPopular = in_array('SHOW_POPULAR', $filters); | ||
|
|
||
| echo "<ul class='jqueryFileTree'>"; | ||
|
|
||
| // Show popular destinations at the top (only at root level when SHOW_POPULAR filter is set) | ||
| if ($rootdir === $fileTreeRoot && $showPopular) { | ||
| $popularPaths = getPopularDestinations(5); | ||
|
|
||
| // Filter popular paths to prevent FUSE conflicts between /mnt/user and /mnt/diskX | ||
| if (!empty($popularPaths)) { | ||
| $isUserContext = (strpos($fileTreeRoot, '/mnt/user') === 0 || strpos($fileTreeRoot, '/mnt/rootshare') === 0); | ||
|
|
||
| if ($isUserContext) { | ||
| // In /mnt/user context: only show /mnt/user paths OR non-/mnt paths (external mounts) | ||
| $popularPaths = array_values(array_filter($popularPaths, function($path) { | ||
| return (strpos($path, '/mnt/user') === 0 || strpos($path, '/mnt/rootshare') === 0 || strpos($path, '/mnt/') !== 0); | ||
| })); | ||
| } else if (strpos($fileTreeRoot, '/mnt/') === 0) { | ||
| // In /mnt/diskX or /mnt/cache context: exclude /mnt/user and /mnt/rootshare paths | ||
| $popularPaths = array_values(array_filter($popularPaths, function($path) { | ||
| return (strpos($path, '/mnt/user') !== 0 && strpos($path, '/mnt/rootshare') !== 0); | ||
| })); | ||
| } | ||
| // If root is not under /mnt/, no filtering needed | ||
| } | ||
|
|
||
| if (!empty($popularPaths)) { | ||
| echo "<li class='popular-header small-caps-label' style='list-style:none;padding:5px 0 5px 20px;'>"._('Popular')."</li>"; | ||
|
|
||
| foreach ($popularPaths as $path) { | ||
| $htmlPath = htmlspecialchars($path, ENT_QUOTES); | ||
| $displayPath = htmlspecialchars($path, ENT_QUOTES); // Show full path instead of basename | ||
| // Use data-path instead of rel to prevent jQueryFileTree from handling these links | ||
| // Use 'directory' class so jQueryFileTree CSS handles the icon | ||
| echo "<li class='directory popular-destination' style='list-style:none;'>$checkbox<a href='#' data-path='$htmlPath'>$displayPath</a></li>"; | ||
| } | ||
|
|
||
| // Separator line | ||
| echo "<li class='popular-separator' style='list-style:none;border-top:1px solid var(--inverse-border-color);margin:5px 0 5px 20px;'></li>"; | ||
| } | ||
|
Comment on lines
96
to
109
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get the full FileTree.php file to understand the context and escaping patterns
wc -l emhttp/plugins/dynamix/include/FileTree.phpRepository: unraid/webgui Length of output: 104 🏁 Script executed: #!/bin/bash
# Read the FileTree.php file, focusing on the area around lines 93-107
sed -n '80,120p' emhttp/plugins/dynamix/include/FileTree.php | cat -nRepository: unraid/webgui Length of output: 2467 🏁 Script executed: #!/bin/bash
# Search for all htmlspecialchars calls and translation patterns in FileTree.php
rg 'htmlspecialchars|_\(' emhttp/plugins/dynamix/include/FileTree.php -n -A 1Repository: unraid/webgui Length of output: 993 🏁 Script executed: #!/bin/bash
# Check how htmlspecialchars is used in other files in the same directory
rg 'htmlspecialchars.*ENT_QUOTES' emhttp/plugins/dynamix/include/ -lRepository: unraid/webgui Length of output: 279 🏁 Script executed: #!/bin/bash
# Check how translations are handled with escaping in similar contexts across the codebase
rg '_\([^)]*\).*htmlspecialchars|htmlspecialchars.*_\(' emhttp/plugins/dynamix/include/ -A 2 -B 2Repository: unraid/webgui Length of output: 2102 🏁 Script executed: #!/bin/bash
# Look for translation followed by escaping pattern (as mentioned in the learning)
rg 'htmlspecialchars.*_\(|_\(.*htmlspecialchars' emhttp/plugins/dynamix/include/ -nRepository: unraid/webgui Length of output: 719 🏁 Script executed: #!/bin/bash
# Check the specific pattern: translated text being escaped in HTML attributes or echo statements
rg -A 2 "echo.*_\(" emhttp/plugins/dynamix/include/FileTree.php -nRepository: unraid/webgui Length of output: 235 🏁 Script executed: #!/bin/bash
# Verify PHP htmlspecialchars default behavior and ENT_QUOTES necessity
python3 << 'EOF'
# Check what htmlspecialchars defaults do
print("htmlspecialchars() default behavior:")
print("- Encoding: ISO-8859-1 (or UTF-8 in modern PHP)")
print("- Flags: ENT_COMPAT (default)")
print("- ENT_COMPAT: converts & < > and double quotes (\") but NOT single quotes (')")
print()
print("For data-path='$htmlPath' attribute context:")
print("- If path contains single quote ('), it will break the attribute")
print("- Example: path = \"/mnt/user's music\"")
print("- Current: data-path='/mnt/user's music' <- BROKEN")
print("- With ENT_QUOTES: data-path='/mnt/user's music' <- SAFE")
EOFRepository: unraid/webgui Length of output: 143 Escape the translated "Popular" text and consider using ENT_QUOTES for attribute escaping. Line 94 outputs - echo "<li class='popular-header small-caps-label' style='list-style:none;padding:5px 0 5px 20px;'>"._('Popular')."</li>";
+ echo "<li class='popular-header small-caps-label' style='list-style:none;padding:5px 0 5px 20px;'>".htmlspecialchars(_('Popular'))."</li>";Additionally, for the - $htmlPath = htmlspecialchars($path);
+ $htmlPath = htmlspecialchars($path, ENT_QUOTES, 'UTF-8');🤖 Prompt for AI Agents✅ Confirmed as addressed by @mgutt
Contributor
Author
There was a problem hiding this comment. I searched in all Unraid WebGUI files and most of them do not use an additional
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: Length of output: 4320 🏁 Script executed: Length of output: 784 @mgutt, you're absolutely right. The data confirms your observation: Translation escaping:
Attribute escaping:
Your new code already follows the established pattern correctly:
I withdraw my suggestions. The code aligns with the project's established conventions. Question: The learnings I retrieved (IDs 🧬 Code Graph Analysis Resultsemhttp/plugins/dynamix/include/PopularDestinations.php ### Lines 132-147function getPopularDestinations($limit = 5) { // Sort by score descending (should already be sorted, but just in case) // Return top N paths return array_map(function($dest) { ⛔ Learnings removed🧠 Learnings used
Contributor
Author
There was a problem hiding this comment. @SimonFair coderabbitai learnt from you, that 
Do you know if translators are allowed to use HTML in their translations (maybe in the future)? @elibosley FYI
Contributor
There was a problem hiding this comment.
|
||
| } | ||
|
|
||
| // Read directory contents | ||
| $dirs = $files = []; | ||
| if (is_dir($rootdir)) { | ||
| $names = array_filter(scandir($rootdir, SCANDIR_SORT_NONE), 'no_dots'); | ||
| // add UD shares under /mnt/user | ||
| foreach ($UDincluded as $name) { | ||
|
|
@@ -89,25 +130,33 @@ function my_dir($name) { | |
| $files[] = $name; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Normal mode: show directory tree | ||
| if ($_POST['show_parent'] == 'true' && is_top($rootdir)) { | ||
| echo "<li class='directory collapsed'>$checkbox<a href='#' rel=\"".htmlspecialchars(dirname($rootdir), ENT_QUOTES)."\">..</a></li>"; | ||
| } | ||
|
|
||
| // Display directories and files (arrays already populated above) | ||
| foreach ($dirs as $name) { | ||
| // Exclude '.Recycle.Bin' from all shares and UD folders from '/mnt' | ||
| if ($name === '.Recycle.Bin' || ($rootdir === $mntdir && in_array($name, $UDexcluded))) continue; | ||
| $htmlRel = htmlspecialchars(my_dir($name).$name, ENT_QUOTES); | ||
| $htmlName = htmlspecialchars(mb_strlen($name) <= 33 ? $name : mb_substr($name, 0, 30).'...', ENT_QUOTES); | ||
| if (empty($match) || preg_match("/$match/", $rootdir.$name.'/')) { | ||
| echo "<li class='directory collapsed'>$checkbox<a href='#' rel=\"$htmlRel/\">$htmlName</a></li>"; | ||
| } | ||
| } | ||
| foreach ($files as $name) { | ||
| $htmlRel = htmlspecialchars(my_dir($name).$name, ENT_QUOTES); | ||
| $htmlName = htmlspecialchars($name, ENT_QUOTES); | ||
| $ext = mb_strtolower(pathinfo($name, PATHINFO_EXTENSION)); | ||
| foreach ($filters as $filter) if (empty($filter) || $ext === $filter) { | ||
| if (empty($match) || preg_match("/$match/", $name)) { | ||
| echo "<li class='file ext_$ext'>$checkbox<a href='#' rel=\"$htmlRel\">$htmlName</a></li>"; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| echo "</ul>"; | ||
| ?> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.