[Bug]: "copy the wikitext to the clipboard" produces different results depending on timing

[mnalis]

Summary

Pressing the button copy the wikitext to the clipboard produces a different result depending if the upload is still in progress (wrong result), or whether the upload has finished (correct result).

Steps to reproduce

1. have a slow network connection
2. add new picture, fill metadata and submit
3. while the picture is still uploading (i.e. a progress bar is being shown), click on the copy the wikitext to the clipboard button and paste the result somewhere
4. wait until the upload finishes
5. click on the copy the wikitext to the clipboard button again and paste the result somewhere

Expected behaviour

the first and second value copied are the same.

Actual behaviour

The first and second results differ. For example, for this file: https://commons.wikimedia.org/wiki/File:Knafel%C4%8Deva_markacija_na_odrezanoj_grani,_Prvi%C4%87.jpg the first copy/paste produces:

[[Knafelčeva markacija na odrezanoj grani, Prvić.jpg|thumb|]]

(which is wrong) while the second copy/paste produces (correct!):

[[File:Knafelčeva markacija na odrezanoj grani, Prvić.jpg|thumb|]]

i.e. the first one misses File: (which then makes problems when one pastes it to the app that expects standard commons format, like e.g. EveryDoor). Especially annoying when the mobile internet is slow, as one is forced to either wait a long time, or manually fix every image name.

Device name

Huawei P30Pro

Android version

Android 10 (EMUI 12)

Commons app version

4.1.0 (latest f-droid)

Device logs

No response

Screen-shots

https://github.com/commons-app/apps-android-commons/assets/156656/71de16af-e770-4aeb-961f-eadd9d19228d

Would you like to work on the issue?

None

[Kshitiz-Mhto]

hello @nicolas-raoul, i am new to the project and exploring the project. Since it is a good-first-issue, can i give it a try?

[nicolas-raoul]

@Kshitiz-Mhto It is yours, thanks! Please let us know about your progress every few days. 🙂

[Kshitiz-Mhto]

actually my exams are ongoing ryt now, so i am giveing as much as free time i have on this. so my response might be delay hope you understand.

i did find this code that casing the issue, i m workng on it. thanks you

file -> fr.free.nrw.commons.media.MediaDetailFragment

[nicolas-raoul]

@Kshitiz-Mhto Sure no worries, please focus on your exams, letting us know every 2 weeks is fine. :-)

[TaiHaDev]

@Kshitiz-Mhto are you still working on this? @nicolas-raoul Can I please give this a try? This is my first open source contribution. I will try my best and I think I would be able to finish this within two weeks 😄

[nicolas-raoul]

@TaiHaDev How about #5263? :-)

[TaiHaDev]

Hi @nicolas-raoul, I had a look at the issue but I couldn't reproduce but I found an interesting one #5212 that I can reproduce the bug . Can I give it a try?😁 I will regularly update my progress. Thank you!

[nicolas-raoul]

@TaiHaDev Sure! Please comment there, as I can't assign you unless you comment. Thanks! :-)

[Kshitiz-Mhto]

my exam r over now, i m all good to go now, i will finish this as soon as i can. thank u!

[mnalis]

Any luck @Kshitiz-Mhto ?

[axelthepony27]

Hello! I'm new to the open source community and exploring the project. I saw this is a good first issue. Could I give it a try?

[annuk123]

Hello! I'm new to the Open Source Contribution

[nicolas-raoul]

@axelthepony27 It is yours, please let us know about your progress every week or so, thanks! 🙂

[nicolas-raoul]

@annuk123 How about https://github.com/commons-app/apps-android-commons/issues/5194 for instance? 🙂

[axelthepony27]

@nicolas-raoul Thanks! I forked the repo and cloned it to my locale, but the file fr.free.nrw.commons.media.MediaDetailFragment appears to have an import that doesn't exist. Is this a problem of mine, or do you know what else could be causing this?

[mnalis]

@axelthepony27 And what happens when you try to compile? Do you get some error, or does it just work?

For me, GitHub workflow seems to compile latest main just fine, e.g. https://github.com/mnalis/apps-android-commons/actions/runs/7346949892/job/20002559077

[axelthepony27]

@mnalis When I try to build, I get this error. Perhaps I missed a step in the configuration? Is there something else I should do? I followed the steps described in the quick guide verbatim (at least to my knowledge).

[mnalis]

@axelthepony27 well, I don't really know (I don't even have local Android SDK installed - I just use GitHub to build it).

But:

• the "failed to find Platform SDK with path" (which is the real error) seems to me unrelated to the originally reported error ("missing fr.free.nrw.commons.BuildConfig", which seems to be a red herring)
• it seems related to your build toolkit, not to Commons app itself. Have you been successfully building any other Android projects on that setup?
• my first guess would be that the "platforms;android-33" part of the error might indicate you need to download that correct platform? e.g. via menu > Tools > SDK Manager ? As shown in screenshot here
• if that doesn't help, I'd try searching the web for the web for similar errors

[axelthepony27]

@mnalis Thanks! Indeed, the problem was solved by installing the correct SDK. I'll get to work on the issue now, it does appear to concern to the fragment of code that @Kshitiz-Mhto mentioned.

[axelthepony27]

Doing some tests, I found out that the "description" metadata also changes if copied during upload. Do we know if this is expected, or is it also part of the issue? In the following SS, the first line of text was copied during upload, and the second one once it was done. We can noties that, indeed, the File: substring is missing, but there are also some dangling {} and en|1= that aren't present in the expected result.

[nicolas-raoul]

Great finding!

Ideally File: should be present even when copied during upload.

The second line's syntax is the right one.

[annuk123]

@annuk123 How about #5194 for instance? 🙂

Thanks 😇

[nicolas-raoul]

@annuk123 That one got taken quickly, but how about https://github.com/commons-app/apps-android-commons/issues/5413 ?

[axelthepony27]

Well, doing some tests, I determined the problem lies on the media.getFilename() and media.getFallbackDescription() methods. Something happens between when the upload is in progress and when it is done that changes the outputs of those methods. I, however, was unable to locate where, as those attributes come from the file Media.kt, and don't have explicit getters. Does anyone have any idea where the problem might originate? Perhaps on the code that manages the upload of files.

[mnalis]

@axelthepony27 perhaps you can add the debug code around places where this gets called to get more precise idea when and where exactly the value changes? That should be instructional to finding the cause of the issue (and thus, the fix).

[axelthepony27]

@mnalis Thanks, I'll try that. I went on vacation for a couple of weeks, but I'll retake the issue this week

[axelthepony27]

Doing some more tests, I noticed that, while an image is still uploading, the "Description" field presents the text in the wrong format, like this:

However, after the image has finished uploading, the description text is correct:

This leads me to believe that the error doesn't lie in the copy method itself, onCopyWikicodeClicked(), as originally thought, but rather on something else: perhaps the the code that generates the text fields on the image's card in the first place. Does anyone know where that happens, so that I could have a better-guided debugging? I can't seem to find where it happends, or another good place to look for the bug.

[axelthepony27]

Well, I found the bit of code that formats the "Description" part only. It seems to happen during upload. The upload process formats the filename and description of an upload differently than when an upload is done. I think after an upload has finished, the details panel pulls the filename and description from some other place. However, I'm unable to find where exactly. In any case, what might be happening could be fixed by one of two ways:

1. Find when and from where the filename and description fields pull the text, and agree them so that they pull the text from only one place (the correct one). I still, however, would need to find when and where it happens exactly.
2. Agree the formats both during upload and when the upload is done. This, however, doesn't seem like a good solution, as the description of method formatDescriptions() in class Contribution.kt says it "Formats the list of descriptions into the format Commons requires for uploads."

Testing it, this method definitively is where the "wrong" format takes place:

I would like a little more guidance on how to proceed, if anyone could lend me a hand.

[Get4nik]

Hi @nicolas-raoul , I just started to look for issues so that I can contribute, since it is a good first issue can i give it a try

[axelthepony27]

Hi, @nicolas-raoul @mnalis. Just following up on this, did you have the chance to check out my questions?

[mnalis]

@axelthepony27 I have too little experience with Commons codebase, so I can't give any specific pointers. If I did have more, I would've taken on the issue myself :smiley:

Regarding your dilemma in where to fix the code, I think it is way too premature way of thinking. Because, one shouldn't try to fix bug the first.

That would be the kludge, and could be done anywhere alongside the path -- e.g. one could then simply check the description in any function before the description is shown to users, and prepend File: before filename if it is missing. Violla, problem solved -- well, sidestepped anyway.

But that is not very good solution. It sometimes might be better than nothing (i.e. it fixes the immediate problem) but codebase maintainers might be unhappy about it (and with good reason: fixing the manifestations of bugs instead of bugs themselves might likely lead to even harder to fix bugs in the future and less readable/understandable codebase -- and even when kludge might be accepted, it should have big bug comment block explaining what is being done in dirty way, and why wasn't it done in better way).

---

But to really solve the bug, one must:

• first be able to reproduce the bug at will. That is often the hardest part, and luckily for us, it seems it is 100% reproducible for this bug, only somewhat time-dependent. That is second-best type of the bug! (the best bug being the one which always happens and does not depend on anything).

• then one must find where the bug is happening. That takes one of the following models, depending on the nature of specific bug:

  • the data is non-existent in program memory, and then (sometimes good, and sometimes bad) data is loaded in program memory (from network, from file, from user input, etc). One should find exact function which does that, and what are its input parameters when it is producing good output, and what are its input parameters when it is producing bad output. Or,
  • the data is existent in program memory in good form (i.e. has File: prefix in one variable) at one place, but then (in some or all cases) at some later time at some other place the data is seen wrong (i.e. it loses File: prefix). One should find exact line of code where such transformation happens. Or,
  • the data is existing in program memory in bad form (i.e. without File: prefix) at one place, but then at the later time at other place it is (sometimes but not always) changed to good form (i.e. with File:) prefix. One should find exact line of code where such transformation happens.

  Here are few ways to how to find where that is happening:

  • I've already suggested one method: using debug logging at various places in the code and then checking the logcat debug output. I don't know if you tried it -- if you did, you should share your findings - i.e. at which line exactly does incorrect data (filename without File:) becomes correct (i.e. has File:).
  • There are other methods which may be more comfortable to you, like using the debugger - e.g. see https://developer.android.com/studio/debug/

  In any case, that should be your fist task - find WHERE the bug happens. That is usually the most time consuming part of chasing reproducible bugs... You say: "However, I'm unable to find where exactly" -- why is that the case? Have you been able to find single place in the code where the value is correct? Have you been able to find single place in the code where the value is incorrect? If you did find those two, have you tried to narrow it down, that is, find other places where value is still correct but closer to place where it is incorrect, or vice versa? Can you detail exactly at what point in this process have you become stuck? I.e. which parts you've managed to do, and at which part of the process have you stuck? If you share that in enough detail, perhaps someone can give specific advice how to overcome that specific obstacle.

• after one has found out where does the bug happen, next task should be to determine WHY the bug happens. Looking at the inputs and outputs and code logic, determine what code path is taken when the output is correct, and what other code path is taken when output is incorrect. Compare them and get understanding what is different and why. If needed, write small example code snippets that replicate what those code paths in simple and short manner if looking at the whole of the code is too complex. That is actually often the easier part to do, when you already know where exactly the bug happens.

• Only after where and why are known, can one try to ponder what would be the best way to FIX it. Sometimes it is simple coding error which needs fixing in already existing code (e.g. some "if" statement might have used "or" instead of "and"), sometimes it is adding some special case handling depending on the input (e.g. if the data has not yet been fully received, wait until it is), and sometimes it needs deeper refactoring (e.g. this class is being used for two different purposes, and it is only suitable for one. So new classes should be made, each which inherits most of the code of the base class, but has special code for its distinct purposes). But we can't know what is best / correct way to fix it until we know where and why is the bug happening.

Anyway, sorry for the length, but I tried to detail it in small and individually understandable pieces to make it easier for you to tackle the problem. Let me know if something is unclear and I can try to explain it better! (Hopefully the effort that went into it can be reused for other usecases, as bug hunting process is often quite similar)

[mnalis]

Hi @axelthepony27 have you made any progress? Are you still intending to work on this?

[mnalis]

@axelthepony27 are you still interested in working on this Commons app issue, or would you prefer to be unassigned so someone else might give it a try?

[mnalis]

@nicolas-raoul: As @axelthepony27 seems MIA (please respond if you're still interested in working on this!), perhaps they could be unassigned so someone else might give it a try ?

[nicolas-raoul]

@axelthepony27 feel free to ask to be re-assigned if you are still working on this. Thanks! :-)