Apache OpenOffice (AOO) Bugzilla – Issue 127897
Possible access to unintended macro in "openoffice/main/cui/source/tabpages/textattr.cxx" line 247
Last modified: 2021-01-13 19:36:20 UTC
While experimenting with a CodeSonar plugin we develop, we noticed a potential issue in file "openoffice/main/cui/source/tabpages/textattr.cxx" line 247 function SvxTextAttrPage::Reset // autogrowsize if ( rAttrs.GetItemState( SDRATTR_TEXT_AUTOGROWSIZE ) != SFX_ITEM_DONTCARE ) { aTsbAutoGrowSize.SetState( ( ( const SdrTextAutoGrowHeightItem& )rAttrs.Get( SDRATTR_TEXT_AUTOGROWHEIGHT ) ). //HERE GetValue() ? STATE_CHECK : STATE_NOCHECK ); aTsbAutoGrowSize.EnableTriState( sal_False ); } Shouldn't SDRATTR_TEXT_AUTOGROWSIZE be used instead of SDRATTR_TEXT_AUTOGROWHEIGHT? Thanks, Petru-Florin Mihancea
Hello Petru-Florin, thank you for your report. Apparently this is not a bug. Or at least, it's a ``known feature'' around which some code is designed. File main/svx/source/sdr/properties/customshapeproperties.cxx at line 47: 47 // change TextFrame flag when bResizeShapeToFitText changes (which is mapped 48 // on the item SDRATTR_TEXT_AUTOGROWHEIGHT for custom shapes, argh) 49 rObj.bTextFrame = 0 != static_cast< const SdrTextAutoGrowHeightItem& >(GetObjectItemSet().Get(SDRATTR_TEXT_AUTOGROWHEIGHT)).GetValue(); That "argh" has been there at least since 2013...
Hi Arrigo, This Property is only valid for Custom Shapes. How does this work for Regular shapes? I find this Code a bit disturbing. Please note that we have a pending PR for this: https://github.com/apache/openoffice/pull/32 I am not really satisfied with the situation. Argh is a good Comment. If this is Correct we should at least add a comment in openoffice/main/cui/source/tabpages/textattr.cxx too. All the best Peter
(In reply to Peter from comment #2) > Hi Arrigo, > > This Property is only valid for Custom Shapes. How does this work for > Regular shapes? I find this Code a bit disturbing. Me too. Looking at openoffice/main/cui/source/tabpages/textattr.cxx, in method SvxTextAttrPage::Construct(), the aTsbAutoGrowSize TriStateBox is enabled if bAutoGrowSizeEnabled is true; and it is true only in case of custom shapes. In those cases, the aTsbAutoGrowHeight TriStateBox is surely disabled. At lines 728-730 aTsbAutoGrowHeight could be enabled, but the condition is always AND-ed with the variable bAutoGrowHeightEnabled set in the Construct() method above. In other words, the "Auto-grow size" and "Auto-grow height" boxes are never enabled at the same time. The reported "strange" mapping (i.e. aTsbAutoGrowSize set based on SDRATTR_TEXT_AUTOGROWHEIGHT) is exactly reversed at lines 424-428: an SdrTextAutoGrowHeightItem is set based on the value of aTsbAutoGrowSize. Petru-Florin may take this file as a good test case for his CodeSonar plugin: two inversions in the same file should have been both reported! ;-) Apart from jokes, I believe that the second one is harder to spot because it involves data types and variable names instead of constants. At least, it took me quite some time to notice it! [...] > I am not really satisfied with the situation. Argh is a good Comment. > If this is Correct we should at least add a comment in > openoffice/main/cui/source/tabpages/textattr.cxx too. I agree with you. Shall we close this bug and continue on the pull request?
I would not close the Issue just yet. But we can continue on Github. That is fine. Whereever we finalize this topic, I just add a short note on the other medium. I think that will help us when we look at this and have no clue anymore what happend.
(In reply to Peter from comment #4) > I would not close the Issue just yet. But we can continue on Github. That is > fine. If we continue here, we have the bug's original poster who can contribute. If we continue on the PR, we have the PR issuer who can contribute. As we started the discussion here, I would like best to try deciding here whether it is a bug or not. Then, once we are finished here, the discussion on the GitHub PR could be about the actual resolution (for example, what wording to use in comments etc.)
To add up more information to my thesis (that this is not a bug), I searched OpenGrok and found that: - SDRATTR_TEXT_AUTOGROWHEIGHT http://opengrok.openoffice.org/search?project=trunk&full=&defs=&refs=SDRATTR_TEXT_AUTOGROWHEIGHT&path=&hist=&type=&si=defs appears in 10 C++ source files, including WW8 documents filters. The corresponding property type, SdrTextAutoGrowHeightItem is used in many more files, some of which call its GetValue() method. http://opengrok.openoffice.org/search?project=trunk&full=&defs=&refs=SdrTextAutoGrowHeightItem&path=&hist=&type=&si=refs - SDRATTR_TEXT_AUTOGROWSIZE http://opengrok.openoffice.org/search?project=trunk&full=&defs=&refs=SDRATTR_TEXT_AUTOGROWSIZE&path=&hist=&type=&si=refs appears in 3 C++ source files, and it only seems as a property that is passed back and forth with dialogs. Even its property type, SdrTextAutoGrowSizeItem, has its GetValue() method never called. http://opengrok.openoffice.org/search?project=trunk&full=&defs=&refs=SdrTextAutoGrowSizeItem&path=&hist=&type=&si=refs If there is a bug here, it is that the "auto-grow size" property is set but never used. The "argh" comment confirms this, i.e. that those shapes for which the property is important, map it on another value.