Created attachment 173720 [details] use NULL for missing XStringKeyMap entries While trying to use SmartTags, the development version (commit f81871ccbbe118b4ac93325a29f7c6b26765cb6e) (but not Version: 6.4.7.2 Build ID: 1:6.4.7-0ubuntu0.20.04.1) reports soffice.bin: file:libo/include/com/sun/star/uno/Sequence.hxx::190 const E& com::sun::star::uno::Sequence< <template-parameter-1-1> >::operator[](sal_Int32) const [with E = com::sun::star::uno::Reference<com::sun::star::container::XStringKeyMap>; sal_Int32 = int]: Assertion `nIndex >= 0 && static_cast<sal_uInt32>(nIndex) < static_cast<sal_uInt32>(getLength())' failed. Stack trace (partial) #3 0x00007ffff7a59f36 in __GI___assert_fail (assertion=0x7ffff3fc4920 "nIndex >= 0 && static_cast<sal_uInt32>(nIndex) < static_cast<sal_uInt32>(getLength())", file=0x7ffff3fc48c8 "libo/include/com/sun/star/uno/Sequence.hxx", line=190, function=0x7ffff3fc4800 "const E& com::sun::star::uno::Sequence< <template-parameter-1-1> >::operator[](sal_Int32) const [with E = com::sun::star::uno::Reference<com::sun::star::container::XStringKeyMap>; sal_Int32 = int]") at assert.c:101 #4 0x00007ffff3d6a443 in com::sun::star::uno::Sequence<com::sun::star::uno::Reference<com::sun::star::container::XStringKeyMap> >::operator[](int) const (this=0x55555b01adb0, nIndex=0) at libo/include/com/sun/star/uno/Sequence.hxx:190 #5 0x00007ffff3d67990 in (anonymous namespace)::SmartTagMenuController::FillMenu() (this=0x55555bdc1400) at libo/svx/source/mnuctrls/smarttagmenu.cxx:128 #6 0x00007ffff3d675f3 in (anonymous namespace)::SmartTagMenuController::statusChanged(com::sun::star::frame::FeatureStateEvent const&) (this=0x55555bdc1400, rEvent=...) at libo/svx/source/mnuctrls/smarttagmenu.cxx:103 I believe this arises because lcl_FillRecognizerData at https://docs.libreoffice.org/sw/html/crsrsh_8cxx_source.html#l03692 file:libo/sw/source/core/crsr/crsrsh.cxx::3663 does not fill aStringKeyMaps, yielding an empty rStringKeyMaps --- How to reproduce - install a smarttags extension - control-click on marked smart tag --- The attached patch avoids the failure by using NULL for missing XStringKeyMap entries. It results in passing NULL to xAction->getActionCount(), xAction->getActionCaptionFromID() and InvokeAction aEntry( xAction, xSmartTagProperties, nActionID ); This seems to match earlier behaviour. It would be nice to be able to pass some info in XStringKeyMap.
On pc Debian x86-64 with master sources updated today, I could reproduce this. (I used tdf#129702 to give it a try because I know nothing about smarttags). About the code pointer, here's the Opengrok link: https://opengrok.libreoffice.org/xref/core/sw/source/core/crsr/crsrsh.cxx?r=7d188fe4&mo=124316&fi=3692#3692
I gave a try with https://gerrit.libreoffice.org/c/core/+/119346 but there's something weird I don't understand as I indicated in gerrit. I don't reproduce this on pc Debian testing x86-64 with LO Debian package 7.0.4 whereas this version has been released at the end of November 2020 and the problematic commit is on 2018. I would have expected to reproduce this on 7.0.4.
With this patch, an Integer stored in XRangeBasedSmartTagRecognizer.recognizeTextRange() can be recovered in XSmartTagAction.invokeAction(). Thank you. > something weird I don't reproduce this on pc Debian testing x86-64 > with LO Debian package 7.0.4 * assertions are probably turned off in debian builds * After looking into the sources: libo/sw/source/core/unocore/unotextmarkup.cxx * SwXTextMarkup supports: getValue, hasValue, insertValue, getCount * It does not support: getKeyByIndex, getValueByIndex I ran into this, storing a value and checking the content with getCount (shows there is something inside), but iterating with getKeyByIndex, getValueByIndex yields dummy values. This might discourage people to use it. These (asserts off, and invokeAction not using the argument) might have masked the problem.
Julien Nabet committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/717ec99667f5a9ab570f1c8581e2d7a0241c32f6 tdf#143464: fix SmartTag management It will be available in 7.3.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-7-2": https://git.libreoffice.org/core/commit/6d799778945e5481a50ffdbb908a188f6ec77aae tdf#143464: fix SmartTag management It will be available in 7.2.0.2. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(In reply to K Antal from comment #3) > With this patch, an Integer stored in > XRangeBasedSmartTagRecognizer.recognizeTextRange() can be recovered > in XSmartTagAction.invokeAction(). Thank you. You're welcome! :-) > > ... > > * assertions are probably turned off in debian builds Indeed but with LO 7.0.4 Debian package, the result is ok. Perhaps because of what you indicate below. > * After looking into the sources: > ... > > These (asserts off, and invokeAction not using the argument) might have > masked > the problem. You may be right indeed.
(In reply to K Antal from comment #3) > ... > * After looking into the sources: > libo/sw/source/core/unocore/unotextmarkup.cxx > * SwXTextMarkup supports: getValue, hasValue, insertValue, getCount > * It does not support: getKeyByIndex, getValueByIndex I think you're rather talking about SwXStringKeyMap instead of SwXTextMarkup
> I think you're rather talking about SwXStringKeyMap instead of SwXTextMarkup Yes.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-7-1": https://git.libreoffice.org/core/commit/41b49f16e7c90c52eeb063f1c4f0ece9b35b51bc tdf#143464: fix SmartTag management It will be available in 7.1.6. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(In reply to K Antal from comment #8) > > I think you're rather talking about SwXStringKeyMap instead of SwXTextMarkup > Yes. Code pointer here: https://opengrok.libreoffice.org/xref/core/sw/source/core/unocore/unotextmarkup.cxx?r=42d2b2d5#515 But reading https://stackoverflow.com/questions/7856453/accessing-map-value-by-index, it seems it's not relevant to get value or key by index since you can't forecast an index. Do you mean we should have a vector in addition to the existing map (maMap is declared as a std::map< OUString, css::uno::Any > ) so we can retrieve the first key or value, the second key or value, etc. ?
> But reading > https://stackoverflow.com/questions/7856453/accessing-map-value-by-index, > it seems it's not relevant to get value or key by index > since you can't forecast an index. My story XSmartTagRecognizer https://api.libreoffice.org/docs/idl/ref/interfacecom_1_1sun_1_1star_1_1smarttags_1_1XSmartTagRecognizer.html promised an XTextMarkup for recognize. XTextMarkup https://api.libreoffice.org/docs/idl/ref/interfacecom_1_1sun_1_1star_1_1text_1_1XTextMarkup.html provides XStringKeyMap getMarkupInfoContainer() and commitStringMarkup ( ... XStringKeyMap xMarkupInfoContainer) The protocol seems to be: - get an XStringKeyMap from getMarkupInfoContainer() - store something in it (what exactly? "additional user defined text markup information" https://api.libreoffice.org/docs/idl/ref/interfacecom_1_1sun_1_1star_1_1text_1_1XTextMarkup.html#a0eec6616a68c7756ad258f5925e4018a ) XSmartTagAction invokeAction() https://api.libreoffice.org/docs/idl/ref/interfacecom_1_1sun_1_1star_1_1smarttags_1_1XSmartTagAction.html#a000c215886a34523e14395c27c540c85 receives xProperties that "Contains the smart tag properties collected by the smart tag recognizer." My questions at this point were / would be: - what does the result of getMarkupInfoContainer() contain? - do I get the same thing at repeated calls? - what can I put in it? What does LO do with it (Which setting will it apply to the tags)? - do repeated calls to commitTextRangeMarkup() require a new call to getMarkupInfoContainer()? Does changing an XStringKeyMap already used in commitTextRangeMarkup() have an effect on the comitted one? To investigate (not all the above, only that I succeded in inserting a value and what is inside), I used XStringKeyMap.getCount(), getKeyByIndex() and getValueByIndex(). And concluded "it does not work". This was not fully correct, but it certainly did not provide all the functionality promised by XStringKeyMap > you can't forecast an index I forecasted the index range: 0 .. (getCount()-1) to be valid, and when iterated over, reveal the content > Do you mean we should have a vector in addition to the existing map (maMap is > declared as a std::map< OUString, css::uno::Any > ) so we can retrieve > the first key or value, the second key or value, etc. ? It would be nice to be able to iterate over the contents, so one can discover what is inside (although it also does not seem necessary for using it). I guess iterating over the contents does not require an extra vector. On the other hand for (int i=0; i<index; i++){ ++iterator; } would be inefficient for large maps. > the first key or value XStringKeyMap does not seem to promise remembering the order of insertion. In case one could put conflicting markup in it, order could matter, but currently I believe this is not the case in terms of LO applying the markup (I am not sure if it is used at all), and I did not have order-preserving in mind. I did not attempt to use it in production, only in discovery. But if it is implemented, someone might. Apart from implementing getKeyByIndex() and getValueByIndex(): *Changing the iterface* I guess changing the XStringKeyMap Interface is out of question. Is it? If some parts were never implemented, probably nobody uses them (since they cannot). I do not know if removing unused parts of an interface is possible without breaking UNO or programs using its other parts. *Documenting* There could be a pointer from XStringKeyMap to the documentation of its sole implementation, or from getMarkupInfoContainer() to the the documentation of the implementation it actually returns, or from recognize to the doc of impl its arguments getMarkupInfoContainer() returns. The documentation of the implementation could contain information on unimplemented parts and other details that can occasionally be important when choosing between implementations (complexity, features not expressed in the interface, like order-preserving). LO API documentation is apparently built around the idea that only the interfaces matter, implementations do not. Sometimes this assumption fails, but we seem to have no place in the documentation to describe it.
I can't respond to your questions, I just don't know :-( About docs, I don't know if it's up-to-date but there's also Opengrok, eg: https://opengrok.libreoffice.org/xref/core/offapi/com/sun/star/text/XTextMarkup.idl?r=2b383d19 However, you might be interested in contributing, here's a link https://wiki.documentfoundation.org/Development/GetInvolved. To summarize, you need to: 1) retrieve source code and build it 2) provide license statement on dev forum (just an email with ad hoc license) 3) create a gerrit account to be able to submit a patch yourself knowing that obviously 2) and 3) have to be done only once. Then you have full access to the code, can study it and do some tests directly. Of course, if stuck, you can let a message on dev LO list or try to ping someone on LO dev IRC (lots of devs are European so have in mind the timezone before calling some advice at 4 AM UTC+0 :-)) Source code of LO is mainly (about 95%) C++ and uses git. Full build, according to the options you choose and power of your pc, it may take several hours.