Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix WebDAV + Thumbnail + Open + Share + port.text + another fix #60

Draft
wants to merge 10 commits into
base: do-webdav
Choose a base branch
from

Conversation

Logic-gate
Copy link

@Logic-gate Logic-gate commented Nov 25, 2023

First of all, thank you for guiding me through the intricacies of git

These fixes include the following:

  • Fix QML rendering error in AddWebDavAccount -- Removal of all _labelItem.opacity: 1 and the addition of port.text

  • Fix Transfer View For Webdav Upload -- This needs to be rechecked since I removed Sailfish.TransferEngine and used Sailfish.Share It works fine, but I am not sure if it's the intention of it, since it only shows cloud services. I'll retest with multiple webdav accounts later today. Looking at the original share page screenshot, it seems that it had Bluetooth share as well. I'll need to recheck this. The problem was that SailfishTransferMethodsModel was throwing an unknown component. I searched for the documentation, but couldn't find anything specific, hence I assumed it was a deprecated method.

  • Fix Webdav Upload -- This entails the change in src/transfers.cpp. It was throwing a Segmentation Fault and fishing for the cause led me to simply select the first element in the list. I suspect it has to do with the changes in UploadFiles.qml

  • Thumbnails

    • Seems like the issue is twofold, for images, the .thumbnails dir is not created. Perhaps it should be done when installing.

    • For videos /usr/lib/qt5/qml/org/nemomobile/thumbnailer/thumbnailers/libvideothumbnailer.so is missing from my system. New implementation requires ffmpeg which I am assuming ships by default with sailfish. If not, then we'll have to re do it using the thumbnail lib. There's a quality factor within ffmpeg: see https://ffmpeg.org/ffmpeg-codecs.html#Options-22.

  • Open fix: Replaced Utilities::openFile which used xdg-open with Qt.openUrlExternally.

I am still working on various fixes and clean ups. You can always check https://ghamama.openproject.com/projects/filecase/work_packages to see where I am and what I am doing

@Olf0 Olf0 self-requested a review November 25, 2023 17:52
@Olf0
Copy link
Collaborator

Olf0 commented Nov 25, 2023

Oh, I am glad to see that posing a PR worked fine for you now.

I am still curious to hear why it did not work in the first place: By searching the net I found that the error message you quoted seems to come from VScode. Were you originally using that and it now worked well using GitHub's web-frontend?

First of all, thank you for guiding me through the intricacies of git

Well, git has a steep learning curve and I am not really that far ahead. What helped me is to take an hour or two at the weekend to read some Git introduction in order to understand basic concepts, the most important being "Git only knows diffs with a timestamp, nothing else. It knows no files proper etc., everything is constructed out of temporally ordered diffs."

Practically I am using a two-pronged approach:

  • GitHub's web-frontend is easy to use and adheres to the usual conventions (much better than GitLab's web-frontend). With a bit of theoretical background one can well see how things are usually done in git.
  • Ultimately, when you face the first conflict-resolution GitHub's web-frontend cannot handle, you have to start using git at the command line. This might initially appear complicated, but the man-pages of git subcommands are well written, and with the aforementioned theoretical background and having experienced how GitHub's web-frontend guides you, it is not that hard.

These fixes include the following:

I will try to look at the WebDAV fixes tomorrow.

A few initial comments, I already have in store:

  • Fix QML rendering error in AddWebDavAccount -- Removal of all _labelItem.opacity: 1

I wonder if they are needed by older SailfishOS releases. Mind that FileCase's code-base currently supports all releases since 1.1.9 (maybe even 1.1.7), that the GitHub CI workflow compiles for some releases starting with 2.2.0 and that SailfishOS Chum supports all releases since 3.1.0. I still have a Jolla 1 with SFOS 2.2.1 and an Xperia X with 3.2.1 around to test.

If you do know for sure that they are superfluous, deleting them is fine, otherwise please comment them out as you originally planned.

and the addition of port.text

I believe this originally was an accidental omission by CepiPerez, but I think it should also be tested for containing something in the line before as all other arguments are, so this unambiguously does not work if port.text is empty.

  • Fix Transfer View For Webdav Upload -- This needs to be rechecked since I removed Sailfish.TransferEngine and used Sailfish.Share

I believe Sailfish.Share replaced Sailfish.TransferEngine in SFOS 4.3.0 or so 4.2.0. We will have to consider how to handle this: Either with version specific branches (as Storeman does) or with ifdefs / ifndefs in the code; the latter is more elegant, as it does not complicate the usual git workflow.

It works fine, but I am not sure if it's the intention of it, since it only shows cloud services. I'll retest with multiple webdav accounts later today. Looking at the share page screenshot(https://raw.githubusercontent.com/sailfishos-applications/filecase/devel/.xdata/screenshots/screenshot-007.jpg) It seems that it had Bluetooth share as well. I'll need to recheck this.

It ought to offer all share providers ("sinks": i.e. "share to"-providers) available on a device. On SFOS 2.2.1 BT is definitely offered and working, will have to check on 3.2.1 and 4.0.1.

The problem was that SailfishTransferMethodsModel was throwing an unknown component. I searched for the documentation, but couldn't find anything specific, hence I assumed it was a deprecated method.

Yes, Jolla loves to replace stuff, often without any migration period (as here with the share mechanism).

  • Fix Webdav Upload -- This entails the change in src/transfers.cpp. It was throwing a Segmentation Fault and fishing for the cause led me to simply select the first element in the list. I suspect it has to do with the changes in UploadFiles.qml

Will take a look at it when time allows.

  • Thumbnails

Oh, this seems to be unrelated to WebDAV, right?
If so, please put these changes into a separate feature branch.
If you are working locally with git at the command line, this can be achieved by creating the new feature-branch from origin:devel (this is a case when things would be easier, if you had retained the original devel branch) and committing only the the thumbnail specific commits (range or list), then reverting these commits from this branch; this is how it theoretically ought to be done: on commit level.
If you prefer to perform this on GitHub's web-frontend, then copy file content with Ctrl-A, Ctrl-C, create a new branch by pasting (Ctrl-A, Ctrl-V) this over the content of the original devel branch (or better: your devel branch after having resynchronised it with the original one) and choosing to create a new branch. The you can restore the original content of these files in the same manner. If you have trouble to find the buttons to click (some are a bit hard to detect on first sight), feel free to ask.

  • Seems like the issue is twofold, for images, the .thumbnails dir is not created. Perhaps it should be done when installing.

Will have a look on my extant FileCase installations.

  • For videos /usr/lib/qt5/qml/org/nemomobile/thumbnailer/thumbnailers/libvideothumbnailer.so is missing from my system. New implementation requires ffmpeg which I am assuming ships by default with sailfish. If not, then we'll have to re do it using the thumbnail lib.

Will check both:

  • What happened to libvideothumbnailer?
  • Is ffmpeg installed by default (IMHO not, but I may be wrong).

There's a quality factor within ffmpeg: see https://ffmpeg.org/ffmpeg-codecs.html#Options-22.

I assume you chose it carefully. 😃

  • Open fix: Replaced Utilities::openFile which used xdg-open with Qt.openUrlExternally.

I am am not sure, if this is correct, because xdg-open is the standard mechanism for opening content via assigned MIME-types, also for local files.

I am still working on various fixes and clean ups. You can always check https://ghamama.openproject.com/projects/filecase/work_packages to see where I am and what I am doing

This is really nice. Please bear in mind to put independent stuff into separate feature branches (ultimately resulting in separate PRs).

@Logic-gate
Copy link
Author

I am still curious to hear why it did not work in the first place: By searching the net I found that the error message you quoted seems to come from VScode. Were you originally using that and it now worked well using GitHub's web-frontend?

I only tried it from the frontend.

I wonder if they are needed by older SailfishOS releases. Mind that FileCase's code-base currently supports all releases since 1.1.9 (maybe even 1.1.7), that the GitHub CI workflow compiles for some releases starting with 2.2.0 and that SailfishOS Chum support all releases since 3.1.0. I still have a Jolla 1 with SFOS 2.2.1 and an Xperia X with 3.2.1 around to test.

If you do know for sure that they are superfluous, deleting them is fine, otherwise please comment them out as you originally planned.

This is an oversight I did not consider. Backwards compatibility with older versions. I'll see if my Jolla 1 or C will boot up and test there.

I believe this originally was an accidental omission by CepiPerez, but I think it should also be tested for containing something in the line before as all other arguments are, so this unambiguously does not work if port.text is empty.

AddWebDavAccount.qml has two calls to createAccount. One in id:pass here
https://github.com/Logic-gate/filecase/blob/2c82ea9682f52843570e998481d64a2b9d92e6a5/qml/pages/AddWebDavAccount.qml#L132 and the second one in id: button here
https://github.com/Logic-gate/filecase/blob/2c82ea9682f52843570e998481d64a2b9d92e6a5/qml/pages/AddWebDavAccount.qml#L143C22-L143C22

There has to be a reason why it is so. I am assuming the port switch is also happening in the back-end side via checking the http string and assigning the port accordingly. I'll rework this so that createAccount has a condition with the basic logic of, if port is not set, assume 443 for https, else 80.

I believe Sailfish.Share replaced Sailfish.TransferEngine in SFOS 4.3.0 or so. We will have to consider how to handle this: Either with version specific branches (as Storeman does) or with ifdefs / ifndefs in the code; the latter is more elegant, as it does not complicate the usual git workflow.

Part of my backwards compatibility oversight. I'll work on it.

It ought to offer all share providers ("sinks": i.e. "share to"-providers) available on a device. On SFOS 2.2.1 BT is definitely offered and working, will have to check on 3.2.1 and 4.0.1.

Yeah, I'll work on this too.

What happened to libvideothumbnailer?
Is ffmpeg installed by default (IMHO not, but I may be wrong).
Not sure to be honest. I am assuming it was renamed libnemothumbnailer. I did try it, but couldn't get it to work. Since ffmpeg is installed by default(Again, i need to conform this). I assumed it would be much better using. I'll recheck them too.

I am am not sure, if this is correct, because xdg-open is the standard mechanism for opening content via assigned MIME-types, also for local files.

Works fine from the command line, refuses to work from filecase. I've spend a good deal of time trying to understand why. Had a look at other projects like file-browser. It seems they too stopped using xdg-open and opted for the Qt solution. In its defense, it's a better solution since it grants the user the choice of selecting which app to open based on the mimetype without much effort. For my X III, it wants to open an App Support app, adding an overhead of starting App Support if it was stopped and then launching whatever app it wants.

This is really nice. Please bear in mind to put independent stuff into separate feature branches (ultimately resulting in separate PRs).

Sure, I can be a bit hectic sometimes. Especially when working on other's code. You dont know where the workflow will take you.

@Olf0
Copy link
Collaborator

Olf0 commented Nov 26, 2023

I only tried it from the frontend.

(still curious) And what was different (rsp. did you do differently) that time when it ultimately worked?

This is an oversight I did not consider. Backwards compatibility with older versions. I'll see if my Jolla 1 or C will boot up and test there.

The question is how much (in the sense of "strongly") one should consider this point. Jolla 1 users are stuck at 3.4.0, but there are only a few left. Community ports are stuck at various releases (the one after which a porter ceased to maintain the port), but I would expect that the sum of all users currently using a "stuck at some release" community port is in the same order of magnitude as all users still using a Jolla 1. My wild guess would be 10 - 100 for each category.
Then there are people like me who cease to install OS upgrades, because Jolla has the consistent tendency to break something with almost every minor release (second field of version string), some Patchmanager-Patches cease to work etc.: This is why my Jolla 1 will always stay at 2.2.1 and my Xperia X at 3.2.1; I still do update apps (both, for SFOS and Android) on them, occasionally.

For all other SFOS-related projects there is a natural cut-off at some release (e.g. by coincidence 3.1.0 for Storeman, SailfishOS-Chum-GUI and consequently their installers, too), but for sfos-upgrade I deliberately support any release since 1.0.0.

For FileCase we would have to decide on a scheme (ifdef- or git branch-based). Then I would like to try to retain compatibility to all SFOS releases, as long it is easy, but as soon it becomes tedious to do so, to make a final FileCase release which also runs on these older SFOS releases, and to introduce breaking changes after that. But I would really like to keep supporting SFOS 3.4.0 for a while.

I believe this originally was an accidental omission by CepiPerez, but I think it should also be tested for containing something in the line before as all other arguments are, so this unambiguously does not work if port.text is empty.

AddWebDavAccount.qml has two calls to createAccount. One in id:pass here https://github.com/Logic-gate/filecase/blob/2c82ea9682f52843570e998481d64a2b9d92e6a5/qml/pages/AddWebDavAccount.qml#L132 and the second one in id: button here https://github.com/Logic-gate/filecase/blob/2c82ea9682f52843570e998481d64a2b9d92e6a5/qml/pages/AddWebDavAccount.qml#L143C22-L143C22

Yes, I saw that, but have not yet taken a closer look why.

There has to be a reason why it is so. I am assuming the port switch is also happening in the back-end side via checking the http string and assigning the port accordingly. I'll rework this so that createAccount has a condition with the basic logic of, if port is not set, assume 443 for https, else 80.

Wasn't already done somewhere above (no time to look at the code right now)?
I would prefer to understand the whole construct before altering it (but this may turn out to be wishful thinking).

I believe Sailfish.Share replaced Sailfish.TransferEngine in SFOS 4.3.0 or so. We will have to consider how to handle this: Either with version specific branches (as Storeman does) or with ifdefs / ifndefs in the code; the latter is more elegant, as it does not complicate the usual git workflow.

Part of my backwards compatibility oversight. I'll work on it.

The switch came with SFOS 4.2.0, this is the reason why Storeman's sfos4.2 branch exists.

It ought to offer all share providers ("sinks": i.e. "share to"-providers) available on a device. On SFOS 2.2.1 BT is definitely offered and working, will have to check on 3.2.1 and 4.0.1.

Yeah, I'll work on this too.

IIRC there is nothing to do specifically for this when using SFOS's transferengine rsp. share mechanism: SFOS should offer all available sinks.

What happened to libvideothumbnailer?
Is ffmpeg installed by default (IMHO not, but I may be wrong).
Not sure to be honest. I am assuming it was renamed libnemothumbnailer. I did try it, but couldn't get it to work. Since ffmpeg is installed by default(Again, i need to conform this). I assumed it would be much better using. I'll recheck them too.

Also still on my ToDo list.

I am am not sure, if this is correct, because xdg-open is the standard mechanism for opening content via assigned MIME-types, also for local files.

Works fine from the command line, refuses to work from filecase.

Huh!?!

I've spend a good deal of time trying to understand why. Had a look at other projects like file-browser. It seems they too stopped using xdg-open and opted for the Qt solution.

O.K.
I know from my Xperia X that xdg-open works fine on SFOS 3.2.1, but that is quite old.

Does "the Qt solution" also work fine for local URLs (file:///)? If so it is just a matter of testing it on older SFOS releases.

In its defense, it's a better solution since it grants the user the choice of selecting which app to open based on the mimetype without much effort. For my X III, it wants to open an App Support app, adding an overhead of starting App Support if it was stopped and then launching whatever app it wants.

Here you lost me: Does "it" address xdg-open or "the Qt solution"?

This is really nice. Please bear in mind to put independent stuff into separate feature branches (ultimately resulting in separate PRs).

Sure, I can be a bit hectic sometimes. Especially when working on other's code. You dont know where the workflow will take you.

Take you time. We are not in a rush, this is all spare time activity.
Actually, I would prefer a slightly slower pace, too (but OTOH do want to avoid stalling your motivation and efforts).

@Olf0
Copy link
Collaborator

Olf0 commented Nov 26, 2023

BTW, on the topic of "using a common core component" for WebDAV: We already do, we all use mhaller's qwebdavlib!

I am considering to use a better way of integrating it as a git submodule. CepiPerez seems to have simply copied the files available at that time (which is pretty close to its final state by mhaller). All aforementioned forks carry only minor changes, though different ones, but mostly not conflicting ones. Mmmh, I did not want to get dragged deeply into FileCase's maintenance aside of infrastructure aspects, but this calls for assembling these changes in a(nother) fork.

@Olf0
Copy link
Collaborator

Olf0 commented Nov 26, 2023

Since ffmpeg is installed by default(Again, i need to conform this). I assumed it would be much better using. I'll recheck them too.

Also still on my ToDo list.

On SailfishOS 3.2.1 ffmpeg-4.1.1 comes from Jolla's RPM repository and is presumably installed by default; I will check the latter for sure and on SFOS 2.2.1.

Edit: ffmpeg was first installed by default on SFOS 3.0.3. Will check if it was available in Jolla's mer-tools repo on 2.2.1.

Edit 2: No, ffmpeg is not at all available on SailfishOS 2.2.1.

All changelogs and release notes of SailfishOS and the Sailfish SDK

As I was originally gathering all SFOS changelogs to determine which packages appeared / vanished / were updated by which SFOS release, a excellent baseline for SFOS 1.1.0.39 is provided by the SailfishOSS page on the MerProject Wiki, which also lists the licenses of all packages (proprietary or a specific FLOSS licence), which are installed by default.

@Olf0 Olf0 changed the base branch from devel to do-webdav November 28, 2023 22:18
@Olf0
Copy link
Collaborator

Olf0 commented Nov 28, 2023

What happened to libvideothumbnailer?

,---
| Sailfish OS 3.2.1.20 (Nuuksio)
'---
[nemo@Sailfish ~]$ ls -l /usr/lib/qt5/qml/org/nemomobile/thumbnailer/thumbnailers/libvideothumbnailer.so 
-rwxr-xr-x 1 root root 9812 Nov 26  2019 /usr/lib/qt5/qml/org/nemomobile/thumbnailer/thumbnailers/libvideothumbnailer.so
[nemo@Sailfish ~]$ rpm -qf /usr/lib/qt5/qml/org/nemomobile/thumbnailer/thumbnailers/libvideothumbnailer.so
nemo-qml-plugin-thumbnailer-qt5-libav-0.0.10-1.3.1.jolla.armv7hl
[nemo@Sailfish ~]$ 
,---
| Sailfish OS 2.2.1.18 (Nurmonjoki)
'---
[nemo@Sailfish ~]$ rpm -qf /usr/lib/qt5/qml/org/nemomobile/thumbnailer/thumbnailers/libvideothumbnailer.so
nemo-qml-plugin-thumbnailer-qt5-libav-0.0.10-1.2.7.jolla.armv7hl
[nemo@Sailfish ~]$ 

Found it in Changelog_from_4.4.0.72_to_4.5.0.16.md at https://forum.sailfishos.org/t/changelog-struven-ketju-4-5-0/14291

============================================
SAILFISH OS
Changelog from 4.4.0.72 to 4.5.0.16
2023-01-30
============================================

# PACKAGES REMOVED

### nemo-qml-plugin-thumbnailer-qt5-libav
- nemo-qml-plugin-thumbnailer-qt5-libav - 0.0.10-1.2.2.jolla

@Olf0
Copy link
Collaborator

Olf0 commented Nov 29, 2023

WRT ~/.thumbnails/

,---
| Sailfish OS 3.2.1.20 (Nuuksio)
'---
[nemo@Sailfish ~]$ devel-su find . -name '.thumb*'
Password: 
[nemo@Sailfish ~]$ 
,---
| Sailfish OS 2.2.1.18 (Nurmonjoki)
'---
[nemo@Sailfish ~]$ find .. -name '.thumb*'
find: ../nemo/.local/share/system/privileged: Permission denied
../nemo/Downloads/.thumbs
../nemo/Pictures/.thumbs
../nemo/Pictures/Papocchio/.thumbs
find: ../nemo/.config/signond: Permission denied
find: ../nemo/.cache/msyncd: Permission denied
../nemo/android_storage/Download/.thumbs
../nemo/android_storage/DCIM/.thumbnails
../nemo/android_storage/DCIM/.thumbnails/.thumbdata3--1967290299
../nemo/android_storage/.thumbs
../nemo/android_storage/paintjoy/.thumbnail
../nemo/android_storage/media/.thumbs
[nemo@Sailfish ~]$ 

FileCase-0.2.0 is installed on both devices.

@Olf0
Copy link
Collaborator

Olf0 commented Jan 22, 2024

Just for an heads up on a meta and social level: Sorry that I have been silent for so long, I was quite ill from early December to early January. I am trying to catch up with the processes at work, private life and SFOS-related activities, but that is overwhelmingly much and I still feel endlessly tired. Hence I am considering to only ask checks&balances-questions here, but to omit all intentions to review code etc., because that will likely not happen anytime soon and I do not want to stall your efforts.

@Olf0
Copy link
Collaborator

Olf0 commented Jan 22, 2024

For an example of the coding differences for the old (< SFOS 4.2.0: Sailfish.TransferEngine) and new (≥ SFOS 4.2.0: Sailfish.Share) sharing mechanism, see Storeman@sfos3.3...sfos4.2.

P.S.: This is also an example of how to handle changes breaking backward compatibility by using different source code branches. For details, see Storeman-Wiki:Git-commit-workflow.
As stated, the theoretically more elegant solution is achiving this by ifdefs / ifndefs (or something similar) in the code and / or spec file.

@Olf0 Olf0 changed the title Fix WebDAV + Thumbnail + Open + Share + another Fix Fix WebDAV + Thumbnail + Open + Share + port.text + another fix Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants