renderer/Document: default to filename if file has no comment #74

Closed

helene wants to merge 1 commit from helene/FoundKey:fix/ap-attachment-names into main

helene commented

2022-08-19 16:02:20 +00:00

Contributor

Filenames will now be used if the DriveFile instance of the attachment has no comment set (which seems to be most cases).

helene added 1 commit 2022-08-19 16:02:20 +00:00

renderer/Document: default to filename

ci/woodpecker/pr/lint-backend Pipeline was successful

Details

ci/woodpecker/pr/build Pipeline was successful

Details

ci/woodpecker/pr/lint-client Pipeline failed

Details

ci/woodpecker/pr/test Pipeline failed

Details

0f3117fca2

Filenames will now be used if the DriveFile instance of the attachment
has no comment set.

toast approved these changes 2022-08-19 16:03:47 +00:00

Johann150 commented

2022-08-19 21:15:10 +00:00

Owner

As far as I can see the name property is not mandatory so I'd like to know why we should ensure it has a value in the first place.

Secondly it is my understanding that the name field is used for image descriptions here. A file name like 2021-10-05 10-04-43 1.png (like files will be named by default) is absolutely not an image description.

As far as I can see the `name` property is not mandatory so I'd like to know why we should ensure it has a value in the first place. Secondly it is my understanding that the `name` field is used for image descriptions here. A file name like `2021-10-05 10-04-43 1.png` (like files will be named by default) is absolutely not an image description.

helene commented

2022-08-19 21:24:19 +00:00

Author

Contributor

As far as I can see the name property is not mandatory so I'd like to know why we should ensure it has a value in the first place.

Neither is the comment property, as FoundKey/Misskey will almost always send out Notes with this field set to null.

Secondly it is my understanding that the name field is used for image descriptions here. A file name like 2021-10-05 10-04-43 1.png (like files will be named by default) is absolutely not an image description.

Usage of the name field as an image description field is abusive and I will submit changes in Pleroma to support the summary field; but that is besides the point. Usage of the name field for image/video descriptions only work for such content types. When sending files that are not images or videos, like PDF files, not setting this field seems counter-intuitive and will result in unnamed files, which I would believe is unexpected behavior. I believe it might be better to have a filename than nothing in any case.

> As far as I can see the `name` property is not mandatory so I'd like to know why we should ensure it has a value in the first place. Neither is the `comment` property, as FoundKey/Misskey will almost always send out Notes with this field set to `null`. > Secondly it is my understanding that the `name` field is used for image descriptions here. A file name like `2021-10-05 10-04-43 1.png` (like files will be named by default) is absolutely not an image description. Usage of the `name` field as an image description field is abusive and I will submit changes in Pleroma to support the `summary` field; but that is besides the point. Usage of the `name` field for image/video descriptions only work for such content types. When sending files that are not images or videos, like PDF files, not setting this field seems counter-intuitive and will result in unnamed files, which I would believe is unexpected behavior. I believe it might be better to have a filename than nothing in any case.

Johann150 commented

2022-08-19 22:01:28 +00:00

Owner

Neither is the comment property, as FoundKey/Misskey will almost always send out Notes with this field set to null.

I don't understand why this is relevant here or makes it necessary for name to always be set in the first place.

Usage of the name field as an image description field is abusive [...].
[...] When sending files that are not images or videos, like PDF files, not setting this field seems counter-intuitive and will result in unnamed files, which I would believe is unexpected behavior. I believe it might be better to have a filename than nothing in any case.

The combination of these two does not make sense to me. If you say that name should not be used for image descriptions why are you still using it for them? Mixing the use of the name field makes it even worse in my opinion. If there is a name and a summary field, the recipient could at least be somewhat sure which is which.

Of course note that if refactoring that, the code for receiving would also have to be adjusted. It would also mean loosing compatibility with Misskey with regard to federating image descriptions.

> Neither is the `comment` property, as FoundKey/Misskey will almost always send out Notes with this field set to `null`. I don't understand why this is relevant here or makes it necessary for `name` to always be set in the first place. > Usage of the `name` field as an image description field is abusive [...]. > [...] When sending files that are not images or videos, like PDF files, not setting this field seems counter-intuitive and will result in unnamed files, which I would believe is unexpected behavior. I believe it might be better to have a filename than nothing in any case. The combination of these two does not make sense to me. If you say that `name` should not be used for image descriptions why are you still using it for them? Mixing the use of the `name` field makes it even worse in my opinion. If there is a `name` *and* a `summary` field, the recipient could at least be somewhat sure which is which. Of course note that if refactoring that, the [code for receiving](https://akkoma.dev/FoundKeyGang/FoundKey/src/commit/e8414e8c8dbb65830877dbb7e381533f886318ac/packages/backend/src/remote/activitypub/models/image.ts#L38) would also have to be adjusted. It would also mean loosing compatibility with Misskey with regard to federating image descriptions.

helene closed this pull request

2022-08-19 22:29:38 +00:00