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
Attachment:
pgplj8rPDCRHI.pgp
Description: PGP signature