profiles: fractal: add ~/.local/share/fractal

[FelixPehla]

I'm on Arch using Fractal 7 and firejail 0.9.72. Even after fixing #6119, using Fractal with firejail would either not work (without Internet access) or show me "Crypto identity incomplete" when using it with, as it stores (encrypted) messages and key material in ~/.local/share/fractal.

This patch fixes that issue for me. Let me know if you'd like me to open an issue first to discuss the problem there before accepting a pull request, or if you have any advice or improvements.

[glitsj16]

Needs additional work:

• add blacklist ${HOME}/.local/share/fractal to disable-programs.inc
• add noblacklist ${HOME}/.local/share/fractal to fractal.profile

[glitsj16]

Thanks for the PR, LGTM now.

[FelixPehla]

Thanks for the looking over it, I amended my previous commit.

I should have probably taken a bit more time to look over the differences between the structure of the last release and master.

By the way, while debugging this I noticed some accessibility-related errors because of missing access to org.a11y.bus and the assistive technology service provider interface (${RUNUSER}/at-spi/bus).

I didn't see a .inc file for accessibility related features, so it might be a good idea to add the option to the profile as well (commented out because of the increased attack surface, with the reference to fractal.local)?

Though I assume people that need this might want to just put it in their global config anyway.

[glitsj16]

By the way, while debugging this I noticed some accessibility-related errors because of missing access to org.a11y.bus and the assistive technology service provider interface (${RUNUSER}/at-spi/bus).

If that is exposed via D-Bus we might consider adding it to fractal.profile:

dbus-user.talk org.a11y.bus

Alternatively we could whitelist ${RUNUSER}/at-spi/bus in the included whitelist-runuser-common.inc. You can test both of these, the latter via a whitelist-runuser-common.local for example.

[FelixPehla]

only adding that still leaves it with (fractal:26): Gtk-CRITICAL **: 22:18:28.795: Unable to connect to the accessibility bus at 'unix:path=/run/user/1000/at-spi/bus': Could not connect: No such file or directory, so I suspect you need whitelist ${RUNUSER}/at-spi/bus (and maybe a noblacklist) too. But I don't know much about those features so I don't know for sure. How would you feel about adding something like this:

# Add the next lines to your fractal.local to allow use of accessibility features
#whitelist ${RUNUSER}/at-spi/bus
#dbus-user.talk org.a11y.Bus

Is there a preferred way of documenting settings with different keyword but related to the same feature? The most elegant way to do this would probably be a conditional, but creating one (and looking where else it should be used) would definitely be out-of-scope of this PR.

[glitsj16]

Thanks for testing a11y. A conditional would be nice, but like you stated, that's out of scope here. Personally I'd leave out any comments regarding a11y and open a separate issue for it so we can add that in later. @kmk3 What do you think?

[kmk3]

@FelixPehla on Jun 23:

I'm on Arch using Fractal 7 and firejail 0.9.72. Even after fixing #6119,
using Fractal with firejail would either not work (without Internet access)
or show me "Crypto identity incomplete" when using it with, as it stores
(encrypted) messages and key material in ~/.local/share/fractal.

This patch fixes that issue for me. Let me know if you'd like me to open an
issue first to discuss the problem there before accepting a pull request

This PR is very simple and self-contained; opening an issue first is more so for
changes that are more complex and/or that affect many files.

or if you have any advice or improvements.

The current commit message title is very long (>72 characters):

$ git show --pretty='%s' -s ba0425fe09
profiles: fractal: mkdir and whitelist ~/.local/share/fractal as it is used for message and key storage
$ git show --pretty='%s' -s ba0425fe09 | tr -d '\n' | wc -m
103

Also, the changes are fairly obvious from the PR title, but for completeness you
could include in the commit message the context from the PR description, such as
the error message.

Suggestion:

profiles: fractal: add ~/.local/share/fractal

Fractal 7 on Arch either does not work (without Internet access) or
shows "Crypto identity incomplete", as it stores (encrypted) messages
and key material in ~/.local/share/fractal.

See the following for details:

• https://chris.beams.io/posts/git-commit/

[kmk3]

@glitsj16 on Jun 23:

Thanks for testing a11y. A conditional would be nice, but like you stated,
that's out of scope here. Personally I'd leave out any comments regarding
a11y and open a separate issue for it so we can add that in later. @kmk3 What
do you think?

Agreed.

The more self-contained a PR is, the easier to review.

The current changes look good as is and the a11y changes seem a bit trickier.

After this PR you could open another one with the a11y changes for fractal and
if it works we could later discuss in a dedicated issue how to implement it for
more profiles.

[FelixPehla]

Thanks for the feedback and the link @kmk3, I'm pretty new to this. I have adjusted my commit message so everything should be good now.

[kmk3]

Thanks for the feedback and the link @kmk3, I'm pretty new to this. I have
adjusted my commit message so everything should be good now.

No problem; the new message looks good.

You made a good attempt to follow the existing conventions and feedback, so
this PR went great.

Looking forward to more contributions :)