Issue 63930

Summary: "if ( bOldVisible && !bOldVisible ) ..." seems meaningless
Product: Draw Reporter: caolanm
Component: codeAssignee: hjs <hans-joachim.lankenau>
Status: CLOSED FIXED QA Contact: issues <issues.openoffice.org>
Severity: trivial    
Priority: P3 CC: frank.schoenheit, issues, mkretzschmar, pavel
Version: OOo 2.0.2   
Target Milestone: OOo 2.1   
Hardware: All   
OS: Linux   
Issue Type: PATCH Latest Confirmation on: ---
Developer Difficulty: ---
Issue Depends on:    
Issue Blocks: 64084    
Attachments:
Description Flags
patch to fix none

Description caolanm 2006-04-03 13:01:22 UTC
svx/source/svdraw/svdpagv.cxx has the above if which doesn't make sense. Patch
attached to remove.
Comment 1 caolanm 2006-04-03 13:01:55 UTC
Created attachment 35465 [details]
patch to fix
Comment 2 wolframgarten 2006-04-03 13:03:17 UTC
Reassigned. Please handle.
Comment 3 pavel 2006-04-03 13:56:21 UTC
set target.

Target OOoLater doesn't make sense for such fixes. if you do not have time to
commit it etc, just approve and reassign back to Caolan.
Comment 4 clippka 2006-04-03 14:01:29 UTC
I disagree. Even if the code seems useless, the patch does not make things better.

It can only make it worse. I have send this issue to Armin because he knows the
code in question and only he can decide if removing the code in is correct or if
the "if ( bOldVisible && !bOldVisible )" is just a typo. If it is a typo, the
code you like to remove is still needed.
Comment 5 clippka 2006-04-03 16:00:48 UTC
wonder what I did, but I didn't send this one to armin so I try again.

Armin, can you please have a look what happened here? can this code be removed?
Comment 6 mkretzschmar 2006-04-03 16:04:33 UTC
The comment in the line after the if certainly sounds as if the intended
condition is something like:

bOldVisible && !bVisible

Anyway, back to lurker mode for me.
Comment 7 Armin Le Grand 2006-05-16 16:06:05 UTC
AW: Changing target
Comment 8 Armin Le Grand 2006-07-10 16:05:37 UTC
AW->FS: Please clarify if change in visibility needs to be handled or not
Comment 9 Frank Schönheit 2006-07-11 13:31:03 UTC
fs->cmc: In fact the piece of code is a dead corpse, which can be removed. The
original intention was surely 
    if ( bOldVisible && !bVisible )
as pointed out by CL. However, the bug 110916 (from the comment below the line)
reads:
  properties for control are not used immediately after change
  1. open a new doc
  2. insert a control
  3. change the background color for the control via property browser
  =>> changes are not displayed until repaint
Well, this obviously doesn't happen in today's builds, so we really don't need
this code, so you're free to remove it.

However, be aware that CWS aw024 (which is due for 2.0.5) completely gets rid of
the complete class, so there's no real need to do this patch now. So, it's up to
you what you want to do with it :)
Comment 10 caolanm 2006-07-11 13:55:22 UTC
cmc->fs: Sure, does the same reasoning hold for the duplicate code in
binfilter/bf_svx/source/svdraw/svx_svdpagv.cxx ? Can that chunk of redundant
logic be removed ? or should it be fixed.
Comment 11 Frank Schönheit 2006-07-13 06:38:36 UTC
(sorry, this comment lingered in my browser for 2 days :-\)

In bifilter, the whole thing is dead (Yet another bunch of zillion lines of code
in binfilter which is completely useless). That is, the SdrUnoControlRecs are a
helper structure for associating UNO controls with their views, but binfilter,
by addition, does never produce views. So, the whole thing there is completely
obsolete (admittedly, finding the right place where to cut code might be tricky).

So, remove it from binfilter if you want, it shouldn't hurt in any way.
Comment 12 caolanm 2006-08-11 15:26:37 UTC
done in cmcfixes26
Comment 13 caolanm 2006-08-14 10:12:29 UTC
reassign for qa
Comment 14 Frank Schönheit 2006-08-15 12:09:40 UTC
VERIFIED that code was removed in binfilter module
Comment 15 caolanm 2007-01-09 11:29:18 UTC
seen in master