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

DL fails if "Download" folder is missing #5829

Closed
ghost opened this issue Oct 7, 2019 · 19 comments
Closed

DL fails if "Download" folder is missing #5829

ghost opened this issue Oct 7, 2019 · 19 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Download good first issue Good for newcomers P2 Upcoming release

Comments

@ghost
Copy link

ghost commented Oct 7, 2019

Trying to download a file will fail if destination directory is deleted, maybe FF should just create a folder if needed, or inform the user why downloading cannot proceed.

┆Issue is synchronized with this Jira Task

@ghost ghost added the 🐞 bug Crashes, Something isn't working, .. label Oct 7, 2019
@boek boek added needs:ac Needs Android Component Work P3 Some future sprint P2 Upcoming release and removed P3 Some future sprint labels Dec 28, 2019
@parvinderandroid
Copy link

I understand that my #13134 is a duplicate and has been closed because of it, but it was far more detailed. Is there a way to bring that description of the bug here?

And why is it taking so long to fix such a small issue? I really wanna use Firefox as I love the new design but I'm stuck using other browsers because of this.

I tried it on Nightly 200905 06:01 today and the issue is still intact.

Video of the issue - https://i.imgur.com/rWRM7dx.mp4

@Amejia481
Copy link
Contributor

Sorry about the delay, we are a small team. we will try to pick the issue as soon as we have some availability. If you are willing to contribute the patch I will glad to help you :)

@parvinderandroid
Copy link

Hey, I am sorry I came out as so entitled earlier. I would love to contribute a patch. Could you please point me in the right direction? I have never made Android apps, but I am good at Java.

@ccapitalK
Copy link

Would this issue be elligible for the good first issue tag? It seems like a fix for this wouldn't require deep understanding of the codebase.

@Amejia481
Copy link
Contributor

Hey, I am sorry I came out as so entitled earlier. I would love to contribute a patch. Could you please point me in the right direction? I have never made Android apps, but I am good at Java.

Do not worry :), we have some docs that can help you to get started. Fenix is the UI of multiple underneath projects Mozilla Android Components (AC) and GeckoView (GV).

The bug is on the feature-downloads component on AC. I think specifically here, where we start to writing to disk and have to check if the folder is there if not we should create it.

These docs can help you:

If you need any additional help just let me know :)

@Amejia481
Copy link
Contributor

Would this issue be elligible for the good first issue tag? It seems like a fix for this wouldn't require deep understanding of the codebase.

I agree it could be tagged as good first issue :)
@parvinderandroid, if you want to proceed and take the issue you could assign yourself to the issue, I will tagged it as "good first issue"

@Amejia481 Amejia481 added the good first issue Good for newcomers label Sep 9, 2020
@parvinderandroid
Copy link

I tried a lot, but I am not able to build fenix when using autoPublish.android-components.dir=../android-components in local.properties

I am able to build and run fenix without local.properties, but it's failing over one reason or the other when I am trying with local.properties.

I don't think I'll be able to do this 😞

@Amejia481
Copy link
Contributor

Is there any specific error that you are encountering? Here you can find a more detail guide of how auto-publication works that may bring some lights, another alternative is to publish manually your Android Component version to Maven Local and set your local Fenix to use it as a dependency.

@parvinderandroid
Copy link

This is what I did:

  1. Clean installed Windows 10 2004 19041.450
  2. Installed all updates and updated to 19041.508
  3. Installed Python 3.8.5 64 Bit
  4. Installed Git 2.28.0 64 Bit
  5. Set Git global user.name and user.email
  6. Installed Android Studio 193.6626763
  7. Set Environment Variable ANDROID_SDK_ROOT=C:\Users\parvi\AppData\Local\Android\Sdk
  8. Set Environment Variable JAVA_HOME=C:\Program Files\Android\Android Studio\jre
  9. Added %JAVA_HOME%\bin to the PATH environment variable
  10. Opened a CMD window in C:\Users\parvi\AppData\Local\Android\Sdk\tools\bin
  11. Ran sdkmanager.bat --licenses
  12. Accepted all 7 Licenses
  13. Opened a CMD window in C:\Users\parvi\Downloads
  14. Ran Download_and_Build.bat >Batch_File_Log.txt
  15. My intention was to direct all output to the text file, but some output was still displayed on the screen that I have attached as a screenshot

Attachments:

Download_and_Build.bat

@Echo on
git clone https://github.com/mozilla-mobile/fenix
git clone https://github.com/mozilla-mobile/android-components
cd fenix
echo autoPublish.android-components.dir=../android-components >local.properties
gradlew clean app:assembleDebug

Batch_File_Log.txt

Output on Screen

@Amejia481
Copy link
Contributor

@parvinderandroid sorry for no replying before, it's been a bumping week for me. I will try to take a look today, I'm not that familiar with dependency substitution (I normally use maven local that could be an alternative), I will ask to other team members on our chat channel on #fenix:mozilla.org channe and #android-components:mozilla.org, please feel free to join the conversation.

@kbrosnan
Copy link
Contributor

The space in the android-components folder seems like a possible problem. The line where that is shown is Could not read script 'C:\Users\parvi\Downloads\android-components \substitute-local-ac.gradle' as it does not exist.

@parvinderandroid
Copy link

The space in the android-components folder seems like a possible problem. The line where that is shown is Could not read script 'C:\Users\parvi\Downloads\android-components \substitute-local-ac.gradle' as it does not exist.

Thank you so much. This might just do it. I am gonna try it as soon as I can.

@parvinderandroid sorry for no replying before, it's been a bumping week for me. I will try to take a look today, I'm not that familiar with dependency substitution (I normally use maven local that could be an alternative), I will ask to other team members on our chat channel on #fenix:mozilla.org channe and #android-components:mozilla.org, please feel free to join the conversation.

No problems at all. I have been a little busy too. I am actually in the final year of my CS degree and have a lot of work to do.

@parvinderandroid
Copy link

The space in the android-components folder seems like a possible problem. The line where that is shown is Could not read script 'C:\Users\parvi\Downloads\android-components \substitute-local-ac.gradle' as it does not exist.

Hey. So I removed the space and tried doing the same steps, the only difference being that I tried it in Windows Sandbox this time.

It still failed. I am attaching the Output on Screen and the Batch File Log:

Batch_File_Log.txt

Screenshot (1)

@Amejia481
Copy link
Contributor

This was addressed on mozilla-mobile/android-components#8785 it should be fixed in the next nightly version.

@Amejia481
Copy link
Contributor

Sorry @parvinderandroid for taking this issue for you, but I had to addressed as the issue was most severe than I initially thought

@Amejia481 Amejia481 removed the needs:ac Needs Android Component Work label Oct 28, 2020
@Amejia481 Amejia481 added the eng:qa:needed QA Needed label Oct 28, 2020
@Amejia481
Copy link
Contributor

QA could you help us to confirm if this issue is fixed on nightly. Thanks in advance!

@parvinderandroid
Copy link

Sorry @parvinderandroid for taking this issue for you, but I had to addressed as the issue was most severe than I initially thought

Hey no problem. This is awesome. What more could I possibly want?

@parvinderandroid
Copy link

QA could you help us to confirm if this issue is fixed on nightly. Thanks in advance!

I just checked and it's definitely fixed. Thank you so much, you guys are awesome!

Proof - https://i.imgur.com/pphDeFD.mp4

@AndiAJ
Copy link
Collaborator

AndiAJ commented Oct 29, 2020

Hi, verified as fixed on the latest Nightly Build 201029 using the following devices:
• Google Pixel 3a (Android 11)
• Huawei Mate 20 Lite (Android 10)
• OnePlus A3 (Android 6.0.1)

► Video
20201029-090645

@AndiAJ AndiAJ closed this as completed Oct 29, 2020
@AndiAJ AndiAJ added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Oct 29, 2020
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 Feature:Download good first issue Good for newcomers P2 Upcoming release
Projects
None yet
Development

No branches or pull requests

7 participants