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
pgpTXxV0ZtXNo.pgp
Description: PGP signature
_______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice