Consider DjVu files to be documents
ClosedPublic

Authored by ngraham on Oct 27 2017, 8:43 PM.

Details

Summary

BUG: 369195

Based on the Wikipedia page (https://en.wikipedia.org/wiki/DjVu), it seems reasonable to consider DjVu files to be documents rather than images.

Test Plan

Tested in KDE Neon. Compiled and deployed fine. .djvu files are now listed as Documents:

Diff Detail

Repository
R293 Baloo
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Oct 27 2017, 8:43 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 27 2017, 8:43 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
ngraham edited the summary of this revision. (Show Details)
rkflx added a subscriber: rkflx.Oct 27 2017, 10:22 PM

General sentiment makes sense, most DjVu documents I've seen were more like books. However, see inline comment. (Someone with actual mimetype or baloo knowledge should approve, though.)

didn't crash baloo with basic usage

If you don't have a DjVu file, that's not "basic usage", but "no usage", making it quite pointless to mention the absence of crashes :)

Anyway, see last section here for some example files: http://www.djvu.org/resources
I guess you need to edit your test plan again…

src/file/basicindexingjob.cpp
219

Looking at /usr/share/mime/image, I see:

  • vnd.djvu.xml: "DjVu image"
  • vnd.djvu+multipage.xml: "DjVu document"

…and that's also what Dolphin shows. Probably best to add +multipage here?

220

Is x-djvu a thing? Not sure, that's why I'm asking.

ngraham updated this revision to Diff 21458.Oct 27 2017, 10:57 PM

Adjusting MIME types to match what's in /usr/share/mime/images

ngraham updated this revision to Diff 21459.Oct 27 2017, 10:58 PM
ngraham marked an inline comment as done.

Actually we don't need the xml extension here

src/file/basicindexingjob.cpp
220

https://www.cuminas.jp/docs/djvuplugin/en_us/Content/MIME%20Types.htm said that it was an older one, so I figured I'd add that one for maximum compatibility. I can remove it if you want.

ngraham edited the test plan for this revision. (Show Details)Oct 27 2017, 10:59 PM
rkflx added inline comments.Oct 27 2017, 11:19 PM
src/file/basicindexingjob.cpp
219

Sorry, with "add +multipage" I meant to add exactly this to the existing line, not to add an additional line. This way, "image" DjVu files are still classified as images by baloo, just as in Dolphin.

This might be inconvenient in some cases, but that's how the mimetype standard defines it.

220

Latest news from djvu.org is from 2013, "older versions of the DjVu Browser Plug-in" are probably even more prehistoric and won't run in upcoming browsers anyway. I don't have this on Tumbleweed, if your Kubuntu does not have it I would tend to removal. Also I believe this is just for opening files in the browser and not relevant to indexing at all.

ngraham updated this revision to Diff 21461.Oct 27 2017, 11:25 PM

Only mark multipage DjVu files as documents, and remove crufty old compatibility mimetype that's probably not relevant at all

ngraham marked 4 inline comments as done.Oct 27 2017, 11:26 PM

Since I was added as a reviewer, I thought I'll comment.

I am not currently maintaining Baloo or using it, so I don't want to really discuss the specifics. Though from a technical point of view this change will work. If nobody has any objections, I would say go for it.

rkflx accepted this revision.Oct 29 2017, 7:26 AM

I'd say this can land. Perhaps best to wait until Monday evening, so Frameworks people not spending their weekend at the computer have a chance to weigh in?

This revision is now accepted and ready to land.Oct 29 2017, 7:26 AM
This revision was automatically updated to reflect the committed changes.