• ImageViewer: Added a "Fit to Width" button. While "Scale to Fit" works fine, I wanted an option that fits the image to width for better readability and scrollability. The "Original Size" option wasn’t cutting it.

Really not fond of that.
You can't just throw a useless-always-there 4th button among 3 valuables ones - just because you want it - or because you read one book with an aspect ratio taller than your device's screen :)
The current button Original/Scale somehow shows the boolean state (at least it shows the "other" state) - your new button makes this crappy.
Possible solution if this is really really needed:

• have the current button cycle Original > Scale > Fit width > Fit height > Original (but I'll be botherered having to cycle more to get back to original).
• or better, now that we have anchored-popup ButtonDialog: show the current state in the button and on tap show these 4 scaling options (with the current one greyed out).
  (Don't implement any of that yet, let us discuss it before.)

Not caring about and not familiar with OPDS stuff: but with streaming, are people really reading books via this ImageViewer (not having all the features of Reader, statistics, pages left...) ? Or is it just for preview ?

Basically lgtm, just a few nits about capitalization and needless comments. (Please keep in mind I only added a few notes explicitly.)

• ImageViewer: Added a "Fit to Width" button. While "Scale to Fit" works fine, I wanted an option that fits the image to width for better readability and scrollability. The "Original Size" option wasn’t cutting it.

Really not fond of that. You can't just throw a useless-always-there 4th button among 3 valuables ones - just because you want it - or because you read one book with an aspect ratio taller than your device's screen :) The current button Original/Scale somehow shows the boolean state (at least it shows the "other" state) - your new button makes this crappy. Possible solution if this is really really needed:

• have the current button cycle Original > Scale > Fit width > Fit height > Original (but I'll be botherered having to cycle more to get back to original).

• or better, now that we have anchored-popup ButtonDialog: show the current state in the button and on tap show these 4 scaling options (with the current one greyed out).
  (Don't implement any of that yet, let us discuss it before.)

Yeah, fair point. I’ll open an issue/enhancement request so we can discuss it properly before making any changes...

Not caring about and not familiar with OPDS stuff: but with streaming, are people really reading books via this ImageViewer (not having all the features of Reader, statistics, pages left...) ? Or is it just for preview ?

Not sure about others, but I use it a lot for reading ongoing manga. Thought it was a minor change, but yeah, it’s kinda crappy :')

Not sure about others, but I use it a lot for reading ongoing manga.

If calling ImageViewer from this specific place, where you'll always be showing a book page for reading - and "fit to width" is always preferable, you maybe could compute and set the right scaling options for fit-to-width directly when you instantiate your ImageViewer ?

"fit to width" is always preferable

It might not necessarily be on something an Elipsa, but I agree. The use case is a bit different from a normal image, so it needn't take all of the regular image defaults.

I've opened an enhancement request for the ImageViewer discussion. Please let me know your suggestions there. I'll probably create a separate PR depending on how the discussion goes.

• If the endpoint doesn’t return proper headers on a HEAD request, the filename was set as empty

Can you show the problematic headers?
Maybe we should fix parsing it, instead of fallback to filename_orig as you propose?

• If the endpoint doesn’t return proper headers on a HEAD request, the filename was set as empty

Can you show the problematic headers? Maybe we should fix parsing it, instead of fallback to filename_orig as you propose?

koreader/plugins/opds.koplugin/opdsbrowser.lua

Line 306 in 154ec62

return header:match('filename="*([^"]+)"*')

The issue wasn’t with how the headers were being matched—they were working fine. The problem was with how the OPDS server was set up. It didn’t support a HEAD request, and the filename was only returned in a GET request.
I know this is a server-side issue that needs to be fixed, but encountering an "is a directory" error when downloading a file wasn’t intuitive from a user experience perspective.
Instead of just falling back, maybe we could show a toast message saying, "No filename found from server; using the original name instead.

That said, I’m okay with skipping this change if there are any objections—I’ve already fixed the server-side issue on my end. :')

To use a server that doesn't return server-filename, a user should uncheck the checkbox and get generated local names.

To use a server that doesn't return server-filename, a user should uncheck the checkbox and get generated local names.

I mean, it does return the filename, but not in the HEAD request—rather, in the GET request, which most browsers and downloaders use by default. But sure, I'll revert this change as well...

in the GET request

Can we use it? In getServerFileName() or in downloadFile()?

in the GET request

Can we use it? In getServerFileName() or in downloadFile()?

I think it should be possible to use it in downloadFile(). It won't work well for getServerFileName(). While we can do a partial GET request with Range, I’m not sure if the all servers support it (it might return 206 without a filename header). Plus, Lua libraries may not handle it well, and we could end up fetching the entire content just to get the header?

For this PR, I’ll revert the changes related to serverfilename and come up with a better approach in another PR—perhaps using HEAD (if supported) or have the filename fetch at downloadFile() method, if that sounds good? But still we would need a fallback name or atleast a toast/InfoMessage saying "failed to fetch filename, please untick raw_file_name from opds feed?"

We shouldn't get the entire content just to have a filename, for sure.
I think, the message that no filename received from the server is enough.

Thanks, can you show the screenshot of the download dialog, to see these special symbols.

Yup... my bad, should've included it before...

Looks good, and the code as well.