Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug]: Filename in "Open" PDF dialog overflows screen #21816

Closed
mcomella opened this issue Oct 8, 2021 · 8 comments
Closed

[Bug]: Filename in "Open" PDF dialog overflows screen #21816

mcomella opened this issue Oct 8, 2021 · 8 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified needs:triage Issue needs triage qa-triaged Issues triaged by qa

Comments

@mcomella
Copy link
Contributor

mcomella commented Oct 8, 2021

Steps to reproduce

  • Visit http://shawnodonnells.com
  • Click "Lunch/Dinner Menu" (may require scroll down)
  • Click "Download"
  • Wait for download to complete

Expected behaviour

Filename fits in dialog

Actual behaviour

Filename overflows "Open" dialog (this doesn't affect earlier "Download" dialog)

device-2021-10-08-162921

Device name

Moto G5

Android version

?

Firefox release type

Firefox Nightly

Firefox version

Nightly 10/8

Device logs

No response

Additional information

No response

┆Issue is synchronized with this Jira Task

@mcomella mcomella added 🐞 bug Crashes, Something isn't working, .. needs:triage Issue needs triage labels Oct 8, 2021
@mavduevskiy mavduevskiy self-assigned this Oct 21, 2021
@LaurentiuApahideanSV
Copy link

I tested the issue on Firefox Preview Nightly 2021-11-02. I was able to reproduce it, however the issue also occurs on Chrome.

Devices used:

  • Oppo Find X3 (Android 11)
  • OnePlus 6T (Android 9)

@LaurentiuApahideanSV LaurentiuApahideanSV added the qa-triaged Issues triaged by qa label Nov 2, 2021
mavduevskiy added a commit to mavduevskiy/fenix that referenced this issue Nov 4, 2021
…creen

Changing the download file name length to the max allowed by AS (251 char, won’t compile if more; max would be 260 for latest windows versions, but generally it is 255), and changing the UI test to check if the long file name is fully visible.
mavduevskiy added a commit to mavduevskiy/fenix that referenced this issue Nov 4, 2021
…creen

Changing the downloaded dialog layout to properly display really long file names
mavduevskiy added a commit to mavduevskiy/fenix that referenced this issue Nov 4, 2021
…creen

Adding a comment to explain the download file name choice
mavduevskiy added a commit to mavduevskiy/fenix that referenced this issue Nov 5, 2021
…creen

Changing the download file name length to the max allowed by AS (251 char, won’t compile if more; max would be 260 for latest windows versions, but generally it is 255), and changing the UI test to check if the long file name is fully visible.
Changing the downloaded dialog layout to properly display really long file names.
mavduevskiy added a commit to mavduevskiy/fenix that referenced this issue Nov 5, 2021
Changing the download file name length to the max allowed by AS (251 char, won’t compile if more; max would be 260 for latest windows versions, but generally it is 255), and changing the UI test to check if the long file name is fully visible.
Changing the downloaded dialog layout to properly display really long file names.
mergify bot pushed a commit that referenced this issue Nov 6, 2021
Changing the download file name length to the max allowed by AS (251 char, won’t compile if more; max would be 260 for latest windows versions, but generally it is 255), and changing the UI test to check if the long file name is fully visible.
Changing the downloaded dialog layout to properly display really long file names.
@Taknok
Copy link
Contributor

Taknok commented Nov 6, 2021

This PR #22325 breaks the default MAX_PATH on Windows (260 chars). Thus, the repo cannot be cloned anymore on Windows 10 if you do not increase the value. (To be precise, main branch cannot be checkout).

To increase the max value, run the following powershell command and reboot:

New-ItemProperty -Path "HKLM:\SYSTEM\CurrentControlSet\Control\FileSystem" `
-Name "LongPathsEnabled" -Value 1 -PropertyType DWORD -Force

Source: https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=powershell

@grigoryk
Copy link
Contributor

grigoryk commented Nov 6, 2021

This PR #22325 break the default MAX_PATH on Windows (260 chars). Thus, the repo cannot be cloned anymore on Windows 10 if you do not increase the value. (To be precise, main branch cannot be checkout).

To increase the max value, run the following powershell command and reboot:

New-ItemProperty -Path "HKLM:\SYSTEM\CurrentControlSet\Control\FileSystem" `
-Name "LongPathsEnabled" -Value 1 -PropertyType DWORD -Force

Source: https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=powershell

Thanks, @Taknok ! That was unintentional, of course.
cc @mavduevskiy can you adjust the test file to be under the limit?

@grigoryk
Copy link
Contributor

grigoryk commented Nov 7, 2021

Should be fixed by #22354

@mavduevskiy
Copy link
Contributor

I am a bit puzzled on what to do. The very idea behind adding a file with such a long name was to test if the app can properly display downloaded file with max possible name length. Which happens to be in the range of 250-260. Which happens to be also the max path name in Windows systems. While on MacOS it is 1024, and on Linux 4096.

Shouldn't the most of developers working on Widnows increase the max path length? Having max path length the same as max file name length on most of the systems is prone to errors. Even if we cut the file name in half, some windows users might still run into the same problem if they have a long project folder path on its own.

Not holding any strong opinions though. Just want to find balance between the need to test displaying of max length file names and adjusting to Windows system requirements.
@Taknok @grigoryk

@Taknok
Copy link
Contributor

Taknok commented Nov 7, 2021

I share your feelings. I report this as a remark, in order to be sure everyone was aware.
Windows is quite a pain to have a path length that short, but preventing someone since the begging to clone (checkout in reality) the main branch or event unzip the archive makes me uncomfortable, but it is a valid choice for Fenix's team. So I see 2 options here:

  • Set a paragraph in the Readme.md to warn for windows env that user/dev should increase max path length and reboot before cloning.
  • Generate the file to download programmatically (thus, the file will not be in the repo) and set a check in graddle to check max path length and ask user/dev to increase the limit. If not done, the test will only failed on windows environment.

Another option would be to change the filename length according to environment, but I do not like this idea.

Joke: But why does anybody sane want to dev on Windows ?

@grigoryk
Copy link
Contributor

grigoryk commented Nov 7, 2021

#22354 landed, I confirmed that this fixed git clone of the project on my Windows machine.

@sflorean
Copy link
Contributor

Verified as fixed on latest Nightly build, with LG G7 Fit (Android 8.1).

@sflorean sflorean added the eng:qa:verified QA Verified label Nov 11, 2021
pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this issue Mar 7, 2022
Changing the download file name length to the max allowed by AS (251 char, won’t compile if more; max would be 260 for latest windows versions, but generally it is 255), and changing the UI test to check if the long file name is fully visible.
Changing the downloaded dialog layout to properly display really long file names.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified needs:triage Issue needs triage qa-triaged Issues triaged by qa
Projects
None yet
Development

No branches or pull requests

6 participants