Desktop: Fixes #12531: Fix the order of attached images (#12531)

[JZou-Code]

What it does

Fixes an off-by-one error in the attachments loop that caused images to render in reverse order.
Insert white space in the expected area.

How to test

1. Open a note with multiple attachments in the desktop app.
2. Verify that they now appear in the order they were added.
3. All existing automated tests pass (yarn test resourceHandlingt).

Related issues

Fixes #12531

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[JZou-Code]

I have read the CLA Document and I hereby sign the CLA

[laurent22]

All existing automated tests pass (yarn test resourceHandlingt).

But how can they pass if the order is flipped? I think it just means there's not test to validate this change and as a result I'm not very confident to merge this.

[q2343454]

Thank you for restoring the fallen black mage back to a white mage!

[JZou-Code]

All existing automated tests pass (yarn test resourceHandlingt).

But how can they pass if the order is flipped? I think it just means there's not test to validate this change and as a result I'm not very confident to merge this.

Yes—I’ve looked through the test suite, and I don’t see anything that verifies insertion order. I tried writing my own tests, but I couldn’t get them to behave as expected—the way the code jumps between helpers and methods is a bit complicated for me to follow.

At the moment, I’ve simply reversed the for-loop inside commandAttachFileToBody. Alternatively, I could leave the existing source untouched and just call filePaths.reverse() before the loop, which also fixes the images-in-reverse issue. Would either of these solutions put your mind at ease? If so, I’ll go ahead and update my PR.

[laurent22]

Any chance you could provide a before/after screenshot? I think that would at least show what it does, even if it's hard to test with automated tests.

[JZou-Code]

Any chance you could provide a before/after screenshot? I think that would at least show what it does, even if it's hard to test with automated tests.

No problem. Here are the screenshots:

Selected Images (Usually expected the insertion order to match the order shown here)

Before the code change (images were sorted in reverse order)

After the code change (images were sorted in ascending order)

[laurent22]

Thanks for looking into this, and I agree it's an annoying behaviour that the system drops the files in reverse order (you could fix this by selecting them in reverse order before dropping them), but I'm not sure your fix would help in all cases or is actually cross-platform.

Maybe we should decide what we want regardless of the order the system is providing. Do we want the files in alphabetical order? Maybe. But then what if someone select their files sorted by creation time for example and expect them to end up in that order?

So I'm not sure what to do here. Maybe your change makes it slightly better, but it might also break someone's workflow, or flip the order in some operating systems, even though it was already in the correct order. Perhaps we need to check that other apps are doing? Is there a strong case for example to always sort the files in alphabetical order? Is that what other apps are doing?

[JZou-Code]

In my opinion, I think this change as keeping the overall workflow intact and simply reversing the array. The change only touches the attachment upload, so in theory it should be safe.

Moreover, In my own usage of apps that support bulk image uploads, I usually rename files alphabetically first. After uploading in bulk, the files appear in the order I expect.

I tested it on both Windows and macOS, and the behavior before vs. after the change is consistent on both systems. But as you pointed out, I can’t ensure this change won’t have any negative impact at all.

That said, I completely understand your caution—this project has many users and contributors, so every change needs care. If this can be accepted, I’d be thrilled, because this is my first PR to a real project as a beginner. If it can’t be merged for any reason, I totally get that as well. Thanks a lot for reviewing my request so patiently.

[tomasz1986]

Do we want the files in alphabetical order? Maybe. But then what if someone select their files sorted by creation time for example and expect them to end up in that order?

Just a comment, and this is only about Windows, but if you order the files using a different criteria in the Explorer, e.g. size, then they are attached to Joplin following that one, i.e. it doesn't need to be alphabetical. However, in either case, the order is still always reversed.

I haven't tested in other applications, but for example in Thunderbird, the order of attachments is correct (= not reversed like in Joplin).

(you could fix this by selecting them in reverse order before dropping them)

This is true, so with this PR, even if you do select the files in another order on purpose, would they still get reversed when attached? Is this right?

[laurent22]

I've worked with applications where I would sort in reverse alphabetical order in Finder before dropping the files, so that they are in correct (alphabetical) order in the app. You can do that for Joplin too.

I'm not sure there's any consistency between apps and even those that are "correct" as far as I can see it just means that internally they sort the files in alphabetical order. For example whether I reverse the order in Finder or not, the files always end up in the same alphabetical order in the app.

But maybe that's reasonable too. Perhaps we should change the app to sort the files before attaching them?

[JZou-Code]

(you could fix this by selecting them in reverse order before dropping them)

This is true, so with this PR, even if you do select the files in another order on purpose, would they still get reversed when attached? Is this right?

I just tested this on Windows: you can sort files however you like when selected them—by date, size, etc.—in ascending or descending order. Whether you select all files or only some of them, and regardless of the order you click them, the files will be attached in the same order they’re displayed in Windows. In other words, whatever the current Windows sort order is, that’s the order the files are added to the editor.

[JZou-Code]

But maybe that's reasonable too. Perhaps we should change the app to sort the files before attaching them?但也许这也合理。也许我们应该修改一下应用程序，让它在上传文件之前先进行排序？

You’re right—defining the order up front when adding files to the array and then inserting them into the editor at the end in that expected order is definitely more reliable. That was my original plan, but I couldn’t pin down the single source of truth for the ordering, so I went with this approach instead. If it helps, here’s the result of a small test I just ran on Windows:

Users can sort files however you like when selected them—by date, size, etc.—in ascending or descending order. Whether you select all files or only some of them, and regardless of the order you click them, the files will be attached in the same order they’re displayed in Windows. In other words, whatever the current Windows sort order is, that’s the order the files are added to the editor.

[laurent22]

That was my original plan, but I couldn’t pin down the single source of truth for the ordering, so I went with this approach instead

I guess sorting should be done where you've currently added your code? @JZou-Code JZou-Code

[JZou-Code]

Sorry, I’ve been tied up with coursework and some difficult personal issues over the last two weeks, so I haven’t had the chance to refine this PR further. I’ll get back to debugging it from here.

[q2343454]

“If it’s a difficult problem, wouldn’t it be a good idea to ask artificial intelligence for bug fixes?
With your skills combined with the support of AI, I think the results could be very good.”

[q2343454]

In version 3.4.10, images are still attached in reverse order.
The issue has not been resolved (crying).

[mrjo118]

@JZou-Code I have not compiled and tested this, but I think you can achieve the required change by doing the following.

Remove all your current changes, then on this line I have commented on (immediately before the for loop) add the following:
const collatorLocale = getCollatorLocale();
const collator = getCollator(collatorLocale);
filePaths = filePaths.sort((a, b) => {
return collator.compare(a, b);
});

This will sort all the file paths alphabetically, while being case insensitive and working correctly with characters from different languages. Presumably it's only possible to select multiple files which are in the same directory, so it should not be necessary to extract the filename only for sorting, as all paths should be the same. Note that you will also need to import the collator using:
import { getCollator, getCollatorLocale } from '@joplin/lib/models/utils/getCollator';

[JZou-Code]

@mrjo118 Hi there—thank you very much for the suggestion. I’d been confused about where and how to sort, and your guidance helped clarify things.

I also tried your approach. After applying sort, all files selected in a single upload end up being fixed to name-based order (just like the situation you mentioned), even if the original selection was sorted by size or creation date. That may not match expectations. By the same token, doing a different fixed sort before the for loop would also fail to preserve the user’s chosen ordering. Adding a configuration object just to pick an ordering feels a bit heavy.

At the moment, my logs show the root cause of the reversed image order: when adding multiple files in one go, options.position doesn’t advance with each inserted link, so every insertion happens at the same index. That produces reversed effect.

[JZou-Code]

@laurent22

Hello, I investigated further and found that when adding multiple files in a single action, the insertion position (options.position) stays unchanged. As a result, the earlier a file is processed in the loop, the farther back it appears in the note—the actual attachment order ends up reversed from the intended order.

Accordingly, my earlier fix was to iterate the for loop in reverse, which cancels out this reversal.

I also tried two alternative approaches:

1. Call filePaths.reverse() before the loop, so the array order counteracts the original reversal.

2. Advance the insertion position by the length of each inserted link after every insertion, so subsequent inserts are appended after the previous one:

	// NewLine: record the initial position
	let pos = options.position ?? 0; 

	for (let i = 0; i < filePaths.length; i++) {
		const filePath = filePaths[i];

		// NewLine: record the body length before new file insertion
  		const beforeLen = body.length; 

		try {
			logger.info(`Attaching ${filePath}`);
			const newBody = await shim.attachFileToNoteBody(body, filePath, pos, {
				createFileURL: options.createFileURL,
				resizeLargeImages: Setting.value('imageResizing'),
				markupLanguage: options.markupLanguage,

				// Modification: insert a white space at the start point if this isn't the first file
				resourcePrefix: i > 0 ? ' ' : '', 
			});

			if (!newBody) {
				logger.info('File attachment was cancelled');
				return null;
			}

			// NewLine: move the position with the length of the length of new file link
			pos += newBody.length - beforeLen; 

			body = newBody;
			logger.info('File was attached.');
		} catch (error) {
			logger.error(error);
			bridge().showErrorMessageBox(error.message);
		}
	}

I tested all three approaches with selections sorted by name, file size, and creation time, and with different file types; they all behaved as expected. If you have a preference among these approaches, please let me know and I’ll update the code accordingly.

[JZou-Code]

“If it’s a difficult problem, wouldn’t it be a good idea to ask artificial intelligence for bug fixes? With your skills combined with the support of AI, I think the results could be very good.”

Hello. The issue itself isn’t particularly difficult. The main challenge is ensuring that users of the modified version won’t encounter other problems—potentially even more serious ones. That’s a point I may struggle to resolve; it’s difficult even with AI, but I’m working on it. Hope it ultimately resolves your issue.

[mrjo118]

I also tried your approach. After applying sort, all files selected in a single upload end up being fixed to name-based order (just like the situation you mentioned), even if the original selection was sorted by size or creation date. That may not match expectations. By the same token, doing a different fixed sort before the for loop would also fail to preserve the user’s chosen ordering

@JZou-Code I may be wrong, but from what I understood from Laurent's last reply, while initially hesitent, I think he was saying that always applying alphabetical sort would be an acceptable solution.

However, as you have found the root cause and proved the ordering is consistently reversed due to a logic error, it does make sense to keep the order the user has selected. Glad I could be of help.

I think your current approach or calling .reverse explicitly are likely ok, as long as you add a comment in the code to explain why it needs to be reversed. Your other alternative is more explicit about the problem without a comment, and is possibly the best choice

[mrjo118]

@JZou-Code Actually for your 2nd alternative solution, does it work if you insert images mid way through the note? If not then one of the other approaches may be better

[JZou-Code]

@JZou-Code Actually for your 2nd alternative solution, does it work if you insert images mid way through the note? If not then one of the other approaches may be better

Yes. I tested inserting in the middle of the note, in the middle of a sentence, and in the middle of a single bullet point. The insertion order appears to match expectations. See the reference images:

1. The first group is sorted by file size (order 8–3–1, which matches the display).
2. The second one is sorted by creation time (order 7-8-1).
3. The other one groups are sorted by name in ascending order.

[mrjo118]

@JZou-Code Actually for your 2nd alternative solution, does it work if you insert images mid way through the note? If not then one of the other approaches may be better

Yes. I tested inserting in the middle of the note, in the middle of a sentence, and in the middle of a single bullet point. The insertion order appears to match expectations. See the reference images:

1. The first group is sorted by file size (order 8–3–1, which matches the display).
2. The second one is sorted by creation time (order 7-8-1).
3. The other one groups are sorted by name in ascending order.

In that case, I'd say go for that option

[JZou-Code]

In that case, I'd say go for that option

Thanks, @mrjo118 . I've implemented with that option and updated this PR accordingly.

[JZou-Code]

Hi @laurent22, I’ve updated the code to use the insert-position approach discussed above and manually verified that the insertion order behaves as expected. If anything looks off or you’d prefer a different direction, please let me know. Thanks!

[laurent22]

@JZou-Code, why not add the files in alphabetical order as discussed previously?

[JZou-Code]

@laurent22 Sorry—I may have misread the requirement and didn’t realize it should be sorted alphabetically.
However, I’ve tested it and can make the list end up sorted by name regardless of whether the original sort is by file name, size, or last modified. Is that the expected behavior for now? If so, I’ll update the code and submit the change right away.

[mrjo118]

@laurent22 Sorry—I may have misread the requirement and didn’t realize it should be sorted alphabetically. However, I’ve tested it and can make the list end up sorted by name regardless of whether the original sort is by file name, size, or last modified. Is that the expected behavior for now? If so, I’ll update the code and submit the change right away.

I think he's made it clear now that he wants you to implement fixed alphabetical sorting, rather than any of the other options you gave

[JZou-Code]

I think he's made it clear now that he wants you to implement fixed alphabetical sorting, rather than any of the other options you gave

Ok, I'll go update the code. Thank you very much.

[JZou-Code]

@laurent22 Hello, I’ve updated the code. The current behavior is that when multiple files are inserted, they are always arranged in ascending alphabetical order by name.

[JZou-Code]

@laurent22 Hi, I’ve attached two GIFs to illustrate the behavior before and after the code changes. As shown, regardless of the order in which attachments are selected, they are now consistently inserted into the note in ascending alphabetical order by name.

Before

After

[laurent22]

That looks good, now thanks for fixing this @JZou-Code!