New issue
Advanced search Search tips

Issue 884557 link

Starred by 1 user
Status: Duplicate
Merged: issue 779414
Owner: ----
Closed: Sep 18
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Drag-and-drop fails to create shortcut for documents with long titles

Reported by bradgraf...@gmail.com, Sep 16 
UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.92 Safari/537.36

Steps to reproduce the problem:
PREREQUISITES:
- Windows 10 (but likely same for 8, 7, Vista...)
- In registry key [HKLM\SYSTEM\CurrentControlSet\Control\FileSystem], LongPathsEnabled set to "1" (but likely same for "0")

1. Visit a webpage with a "long" title (more concrete definition below). Example: https://www.amazon.com/gp/video/detail/B01691N5KK/
2. Drag-and-drop the icon to the left of the URL in the address bar/omnibox (i.e. the padlock for the above example) to a location in Windows Explorer (i.e. the desktop) that would result in a path that would exceed Windows' MAX_PATH constant (260). Assume the filename will be the document's title plus ".lnk".

What is the expected behavior?
A shortcut is created in the target folder using the document title with truncation; either up to ~32768 characters (assuming the Unicode API is used) or up to ~260 characters (assuming the older Windows API is used).

What went wrong?
No shortcut was created at all.

Did this work before? N/A 

Chrome version: 69.0.3497.92  Channel: stable
OS Version: 10.0
Flash Version: 

Unsure if the Windows API is throwing an error/exception that Chrome is silently catching and discarding rather than reacting by truncating the target shortcut's filename. Either way, a silent failure is not very UX-friendly. Prompting for a filename seems too clunky. Consider automatically using truncation (with a target of the MAX_PATH, 260, for maximum compatibility). For example, the filename logic becomes (JS code for illustration; yes, I know it's not C, sorry...):

    const targetDir        = "C:\\path\\to\\dropped\\folder";
    const pathSep          = "\\";
    const MAX_PATH         = 260;
    const NULL_TERM        = "\0"
    const targetFileSuffix = ".lnk";
    let targetFileName     = ExistingSanitizeFunc(document.title);

    // new suggested logic:
    const maxTargetFileLen = MAX_PATH - (targetDir.length + pathSep.length + targetFileSuffix.length + NULL_TERM.length);
    const newTargetFileLen = Math.min(targetFileName.length, maxTargetFileLen);

    let targetFilePath = targetDir + pathSep + targetFileName.substring(0, newTargetFileLen) + targetFileSuffix + NULL_TERM;

Reference: https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#maxpath

Comment 1 by bradgraf...@gmail.com, Sep 16 

Insignificant correction; replace ".lnk" with ".url" in the issue body above.

To demonstrate the boundary for anyone trying to reproduce this, see two attached barebones .html files and the following...

Steps:

1. Download both .html files and place them anywhere that is convenient.
2. Create a directory at the root of a drive that is one character in length (e.g. "C:\a\", "X:\z\", etc.).
3. Open the directory created in step 2 in Windows Explorer (i.e. double-click it).
4. Open the "working.html" attachment in Chrome(ium) browser.
5. Click-and-drag the (i) icon to the left of the file path in the address bar into the Explorer window opened in step 3.
6. Open the "not_working.html" attachment in Chrome(ium) browser.
7. Repeat step 5

Based on this bug, the folder created in step 2 should only contain a single shortcut (ending in the letter 'B'). Step 7 will silently fail to do anything, because the path (e.g. "C:\a\AAAA...BC.url" plus null terminator) will be 261 characters in length, whereas the path+NULL for the shortcut produced in step 5 is exactly 260 characters in length.
working.html
316 bytes View Download
not_working.html
317 bytes View Download

Comment 2 by phanindra.mandapaka@chromium.org, Sep 17

Labels: Needs-Triage-M69

Comment 3 by bradgraf...@gmail.com, Sep 17 

FWIW, now that I decided to take a peek at the code... it seems like CreateValidFileNameFromTitle() in ui/base/dragdop/os_exchange_data_provider_win.cc is attempting to do this type of truncation already... however I don't think it's accounting for the full file path (not just file name).

I tried to dig deeper by preparing my own debug build locally, but clang asserted with the same message as in  Issue 884427 . =/

Comment 4 by dtapu...@chromium.org, Sep 17

Components: Blink>DataTransfer

Comment 5 by bradgraf...@gmail.com, Sep 18 

Okay, worked around my building issue (but created  Issue 885008  because of it) so I could learn more about how these drag-and-drop operations are handled. Now that I have...

I see that my original suggestion won't work. For example, I learned that Chrome doesn't actually know where the dragged shortcut went - it only sets the filename as soon as the dragging starts and lets the receiving window handle the incoming data (i.e. by explorer.exe creating a shortcut with the given filename). Seems like there are three options:

1. Implement the IDropSourceNotify interface to react to DragEnterTarget() notifications as the user is dragging to store the window handle. Just before the drag operation is completed in DragSourceWin::QueryContinueDrag(), check if last saved window handle is a shell window. If so, get its path and re-calculate the max filename length (truncating the name in the OSExchangeData object as needed).
2. Determine some arbitrary number to subtract from MAX_PATH to account for an average path length... where "average path length" is likely just another arbitrary number someone pulls out of a hat.
3. Call this a WontFix issue and hope webmasters stop using long document titles (looking at you, Amazon).

Option 1 is a non-trivial code review for what I'm guessing is a low-priority issue. Option 2 isn't a one-size-fits-all solution, but it would at least be a step in the right direction (IMHO) away from Option 3.

Comment 6 by jdonnelly@chromium.org, Sep 18

Mergedinto: 779414
Status: Duplicate (was: Unconfirmed)
►

Sign in to add a comment