Issue 124372 - the tooltip is "Number %NUMBERINGSAMPLE"
Summary: the tooltip is "Number %NUMBERINGSAMPLE"
Status: RESOLVED FIXED
Alias: None
Product: General
Classification: Code
Component: ui (show other issues)
Version: 4.0.0
Hardware: All All
: P3 Minor (vote)
Target Milestone: 4.1.15
Assignee: AOO issues mailing list
QA Contact:
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2014-03-07 05:49 UTC by zhaoshzh
Modified: 2023-05-23 16:39 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: 4.1.14
Developer Difficulty: ---


Attachments
tooltip is not correct. (250.68 KB, image/jpeg)
2014-03-07 05:49 UTC, zhaoshzh
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description zhaoshzh 2014-03-07 05:49:35 UTC
Created attachment 82806 [details]
tooltip is not correct.

(1) create a slide
(2) input text in textbox,set numbering property
(3) when setting numbering type property, move mouse on the first of numbering type.

the tooltip is "Number %NUMBERINGSAMPLE"
Comment 1 Oliver-Rainer Wittmann 2014-03-07 12:20:42 UTC
I can confirm this defect.
I reproduced it under Windows 7 in AOO 4.0.0, AOO 4.0.1 and AOO 4.1.0 Beta. It is also reproducible in the 'Bullet and Numbering' format dialog in Writer.
In AOO 3.4.1 and before the tool tip is correct - namely 'Number 1) 2) 3)'
Comment 2 Oliver-Rainer Wittmann 2014-03-07 12:23:35 UTC
In the corresponding pop-up control in the Sidebar's Properties Deck, Panel Paragraph the tool tip for the corresponding numbering type is correct.
Comment 3 Andre 2014-03-14 08:19:02 UTC
The tooltip string can be found in svx/source/dialog/svxbmpnumvalueset.src.  The resource entry that now has the content "Number %NUMBERINGSAMPLE" is RID_SVXSTR_SINGLENUM_DESCRIPTION_0.  It was changed for bug 121794 for the migration of the paragraph property panel.
Comment 4 Andre 2014-03-14 09:08:44 UTC
The change looks like a verbatim copy of Symphony code without making
too many necessary adaptions.

What was done is:

- Change the content of the resource string RID_SVXSTR_SINGLENUM_DESCRIPTION_0 from "Number 1) 2) 3)" to "Number %NUMBERINGSAMPLE".

- Create new class NumberingTypeMgr for accessing information related
  to numbering and bullets.  Basically not a bad idea.

- The NumberingTypeMgr is defined in the cryptically named file
svx/source/sidebar/nbdtmg.cxx file with a code qualitiy that is
hard to describe without resorting to unprintable characters like
 = Constructors that call Init() methods twice.
 = ImplLoad methods loading files with such descriptive names as
'standard.sya', 'standard.syb' and 'standard.syc'.
 = These files do not exist in our installation set.
 = UCB is used to read those non-existing files but checks file
existence only when opening a file for writing.
 = The UCB stream is not tested by NumberingTypeMgr for validity but
thankfully it reads one integer and tests it to be a valid version
number, which it isn't.  Reading is aborted before anything worse can
happen.
 = The only places where those files would be written are triggered
during the file reading, i.e. never.

- The numbering style names are read in NumberingTypeMgr::Init where
  the (single occurance) of "%NUMBERINGSAMPLE" is replaced with a more
  meaningful content.

- The idea behind this replacement in Init() seems to be to create
  descriptive strings for a modifyable set of numberings.  Currently
  it is only applied to a fixed set of 8 default numberings.

- The template numbering name is wrongly accessed via
  RID_SVXSTR_SINGLENUM_DESCRIPTIONS which is accidentally identical to
  RID_SVXSTR_SINGLENUM_DESCRIPTION_0.  Using
  RID_SVXSTR_SINGLENUM_DESCRIPTION_0 would be the right way to do it.
Comment 5 Andre 2014-03-14 09:14:34 UTC
This is bad at so many levels that it is really hard to understand:

- The original Symphony code obviously had the ability to handle lists of numbering and bullet styles that where modifiable and extensible at run-time.

- None of this flexibility has been ported to AOO but the code for it has been copied over, now mostly dead.

- One side effect of the Symphony modifications was the introduction of a new resource string that was a template instead of a fixed name.

- The problem with this is that the template was not added but that it replaces and existing string that is still in use in AOO.


Possible fixes:

A. Make the number and bullet style dialog use the NumberingTypeMgr for obtaining the tooltip texts.

B. Revert the modified resource string back to its original value and create a new one for the template.
Comment 6 Oliver-Rainer Wittmann 2014-03-14 09:29:33 UTC
(In reply to Andre from comment #5)
> This is bad at so many levels that it is really hard to understand:
> 
> - The original Symphony code obviously had the ability to handle lists of
> numbering and bullet styles that where modifiable and extensible at run-time.
> 
> - None of this flexibility has been ported to AOO but the code for it has
> been copied over, now mostly dead.
> 
> - One side effect of the Symphony modifications was the introduction of a
> new resource string that was a template instead of a fixed name.
> 
> - The problem with this is that the template was not added but that it
> replaces and existing string that is still in use in AOO.
> 
> 
> Possible fixes:
> 
> A. Make the number and bullet style dialog use the NumberingTypeMgr for
> obtaining the tooltip texts.
> 
> B. Revert the modified resource string back to its original value and create
> a new one for the template.

+1 for B inclusive clean-up of the messy code which might result in having no additional resource string for the template
Comment 7 Andre 2014-03-14 13:15:59 UTC
The code in nbdtmg.cxx is even worse than I thought:

- The classes BulletsTypeMgr, GraphyicBulletsTypeMgr, MixBulletsTypeMgr, NumberingTypeMgr and OutlineTypeMgr are created via static GetInstance() factory methods but their contstructors (including copy constructors) are all public.

- The GetInstance() methods are accessed via the NBOutlineTypeMgrFact factory (which is the first class that I see, that does not need a virtual destructor but still has one).  All implementing classes are publicly visible not just the base class ("interface") NBOTypeMgrBase.

- Some of the classes have static members, although they are essentially singletons and static themselves.

- SwTextShell::ExecSetNumber calls NBOTypeMgrBase::SetItems(const SfxItemSet*) with a pointer to a local SfxItemSet object.  This pointer is stored in the called (static!) NBOTypeMgrBase object until another call to SetItems().  It is only luck  that that pointer is only accessed by functions called inside SetItems.  This is desaster waiting to happen.
Comment 8 Andre 2014-03-17 10:58:43 UTC
And it is even worse:

- When a numbering style is applied via the 'Numbering' button in the 'Paragraph' sidebar panel then for some reason the numbering manager is called back on its SetItems() method.

- One of the side effects of the SetItems() method is that the meCoreUnit member is set.  It is set to 100th mm for Draw/Impress documents and twips for Writer documents.

- Put that together with the numbering manager being a singleton that is shared between application windows, then that can have unexpected consequences:
  > change numbering style in a Writer document
  > meCoreUnit is set to twips
  > change to Draw document
  > Apply some value of the numbering manager that uses the meCoreUnit
  >    uses twips instead of expected 100th mm.
Comment 9 Matthias Seidel 2023-05-23 15:09:38 UTC
Thank you Andre for the deep analysis.

For the moment I have now reverted the string and commented out the code in the sidebar.
If anyone wants to fix and reactivate the code feel free to do so!

Committed to trunk with:
https://github.com/apache/openoffice/commit/f1575a5807415147104696f782205b008ead87c1

Cherry-picked for AOO42X with:
https://github.com/apache/openoffice/commit/fbfb01de6dccdac727b6db620c057a03a40328d7
Comment 10 Matthias Seidel 2023-05-23 16:39:17 UTC
Cherry-picked for AOO41X with:
https://github.com/apache/openoffice/commit/fb13f12c293a1f5f3e8220afc6244b057dfe7610

Various SDF files were patched afterwards, feel free to contact me if you want a language to be added.