Bug 104480 - QML mime type and detection needs work
Summary: QML mime type and detection needs work
Alias: None
Product: shared-mime-info
Classification: Unclassified
Component: freedesktop.org.xml (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: Shared Mime Info group
QA Contact:
Depends on:
Reported: 2018-01-03 15:06 UTC by s@ecloud.org
Modified: 2018-10-13 10:41 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

fix the QML magic (3.95 KB, patch)
2018-01-03 16:26 UTC, s@ecloud.org
Details | Splinter Review
revised: make the shebang detection like the others (3.95 KB, patch)
2018-01-03 16:48 UTC, s@ecloud.org
Details | Splinter Review
revised: more specific about imports (4.73 KB, patch)
2018-01-04 09:08 UTC, s@ecloud.org
Details | Splinter Review

Description s@ecloud.org 2018-01-03 15:06:43 UTC
See also https://bugreports.qt.io/browse/QTBUG-37420 and https://bugreports.qt.io/browse/QTBUG-64435 

1) I think it should be application/qml rather than text/x-qml, but does that depend on having an IANA registration done first?

2) I want to work on the magic, so that "file" can identify a QML file.  But it's kindof hard to be precise... there should be an "import something", and there should be a capitalized word followed by braces.  A shebang at the top is optional.  Is that enough to go on?  Should I just try making a patch and then attach it if I get it working?
Comment 1 s@ecloud.org 2018-01-03 15:53:45 UTC
OK the magic is supposed to be there but it's not working on my Arch Linux system with the /usr/share/file/misc/magic.mgc as distributed in the file package.

  <mime-type type="text/x-qml">
    <_comment>Qt Markup Language file</_comment>
    <magic priority="80">
      <match type="string" value="import Qt " offset="0:256"/>
    <glob pattern="*.qml"/>
    <glob pattern="*.qmltypes"/>
    <glob pattern="*.qmlproject"/>

It expects a space after "import Qt "?  But usually you import QtQuick.  The QML engine doesn't require working only with Qt libraries; for example http://developer.blackberry.com/native/documentation/dev/integrating_cpp_qml/ has import bb.cascades  

Within 256 bytes from the start?  Most license headers are longer.  For example the BSD license in some of Qt's QML files is about 2448 bytes.  So maybe we'd better round up to 3000 or 4096 or so to be safe.
Comment 2 s@ecloud.org 2018-01-03 16:26:00 UTC
Created attachment 136525 [details] [review]
fix the QML magic

I'm not sure if this is quite enough constraint to avoid false positives, but at least I don't see any false negatives.  Tested all qml files on my system: 

 locate -0 "*.qml" | xargs -0 ../xdgmime/src/test-mime | grep -v x-qml
Comment 3 s@ecloud.org 2018-01-03 16:48:01 UTC
Created attachment 136526 [details] [review]
revised: make the shebang detection like the others
Comment 4 David Faure 2018-01-03 20:33:05 UTC
This is invalid XML:

freedesktop.org.xml.in:1868: parser error : AttValue: " or ' expected
      <match type="string" value="/bin/env qml" offset=2:16/>

After adding the missing double quotes, it compiles, but testing it confirms what I suspected: when using magic only, this now detects python scripts as text/x-qml, as long as the python script has a "{" somewhere and an import statement, like this shortened testcase:

#!/usr/bin/env python
import sys, os
args = {}

How about we make it "import Qt", which matches QtQuick, Qt3D, etc.?
What's left is bb.cascades, but well, that's dead isn't it?
(and there's always matching by extension anyway)
Comment 5 s@ecloud.org 2018-01-04 07:16:27 UTC
Comment 6 s@ecloud.org 2018-01-04 08:50:00 UTC
Well the qmlproject files start with

import QmlProject 1.1

so "import Q" matches it.  But we could still get false positives with PySide applications I suppose.  Maybe better to have more specific matches then?
Comment 7 s@ecloud.org 2018-01-04 09:08:57 UTC
Created attachment 136543 [details] [review]
revised: more specific about imports
Comment 8 David Faure 2018-01-05 10:32:37 UTC
Looks good.

I'll add 
args = {}
to the pyside example to make sure that even with a '{' somewhere it still doesn't match the qml magic.
Comment 9 David Faure 2018-01-05 10:33:16 UTC
Pushed in fb8f959, thanks for the contribution.
Comment 10 s@ecloud.org 2018-01-05 14:05:06 UTC
Excellent, thanks for merging.
Comment 11 Bastien Nocera 2018-01-05 14:21:07 UTC
(In reply to David Faure from comment #9)
> Pushed in fb8f959, thanks for the contribution.

The offsets are far too big for normal usage. All the extents fit into about 1k (adding some debug to the extents API in xdgmime would help pin down the exact value), so the ones in the patch should be lowered.

In the worst case scenario, there's a suffix fallback, so there's really no need for such deep magic searches.
Comment 12 s@ecloud.org 2018-01-26 07:13:39 UTC
As mentioned before, QML files often start with copyright messages, so I found the longest one I could find in use, and tried to make it search a bit longer than that.  So maybe the test case should have one of those?  Or maybe it's a waste of space in the repo...
Comment 13 GitLab Migration User 2018-10-13 10:41:25 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xdg/shared-mime-info/issues/84.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.