[Libreoffice] [REVIEW][3-5][3-5-0] Honor pre-selected filter type

[Eike Rathke]

Hi Kohei,

On Friday, 2012-01-27 17:08:14 -0500, Kohei Yoshida wrote:

> I'd like to have the attached patch pushed to the 3-5 branch and
> preferably to the 3-5-0 branch as well.
> 
> It fixes 
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=45084

It seems to fix the symptom and the document is loaded in Calc, but in
impl_detectTypeDeepOnly() case "c)" still "writer_web_HTML" is returned
as sDeepType. queryTypeByDescriptor() attempts to set that via
impl_checkResultsAndAddBestFilter(), which correctly refuses because
a filter was preselected, but then in
impl_validateAndSetTypeOnDescriptor() is

    rDescriptor[::comphelper::MediaDescriptor::PROP_TYPENAME()] <<= sType;

with sType=="writer_web_HTML", and that is also returned by
queryTypeByDescriptor().

To me this works only by chance and would fail where the result of
XTypeDetection::queryTypeByDescriptor() was actually used, e.g.
filter/qa/complex/filter/detection/typeDetection/TypeDetection.java
might if it tested for this scenario.

I also have the impression that case "c)" shouldn't even be reached for
a preselected filter, and instead case "b)" should deliver a hit.

Unfortunately that code is fragile and complex, I didn't reach full
understanding in the 2 hours of reading and stepping through..

> I haven't committed this yet to master, since I wanted to have someone
> else's opinion first.  To the best of my knowledge this change makes
> sense.  Why clear the type and filter just because one of the type
> detections fail?

I think this is to clear filters that make no sense with the given file,
but does not take into account that a preselected filter is to be
cleared only if all type detections fail, or some such..

> Anyhow, review and sign-off appreciated if appropriate.  If not,
> suggestions welcome.

Would be good to sort out the reason why "b)" isn't a hit and
"writer_web_HTML" is still matched even with a preselected matching
filter.

> Oh, BTW, I've already tested the case of opening an html file having a
> xls extension.  It still works after my change.

Adding a test case to the TypeDetection.java mentioned above might
reveal mismatch, somewhere in preselectedFilter.csv or
preselectedType.csv

  Eike

-- 
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20120130/04946d99/attachment-0001.pgp>