FoundKeyGang/FoundKey
FoundKeyGang
/
FoundKey
7
19
Fork 22
Code Issues 37 Pull Requests 4 Releases Activity

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

Closed
helene wants to merge 1 commits from helene/FoundKey:fix/ap-attachment-names into main
pull from: helene/FoundKey:fix/ap-attachment-names
merge into: FoundKeyGang:main
Conversation 3 Commits 1 Files Changed 1 +1 -1
helene commented 2022-08-19 16:02:20 +00:00
Contributor
Copy Link

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

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
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
renderer/Document: default to filename 
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
Copy Link

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
Copy Link

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
Copy Link

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
Some checks failed
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

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
toast
Labels
Clear labels   
feature

task that proposes new feature(s)

  
fix

task to fix behavioral problems in the FoundKey codebase

  
upkeep

task that should make FoundKey smaller and more maintainable

No Label
feature
fix
upkeep
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: FoundKeyGang/FoundKey#74
No description provided.
Delete Branch "helene/FoundKey:fix/ap-attachment-names"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?