Issue 124741

Summary: Slide will be inserted in wrong place when try to insert it in latter slides(with vertical scrollbar)
Product: Impress Reporter: fanyuzhen
Component: editingAssignee: AOO issues mailing list <issues>
Status: RESOLVED FIXED QA Contact:
Severity: Normal    
Priority: P2 CC: awf.aoo, damjan, elish, jsc, liushenf, mseidel
Version: 4.1.0-devKeywords: regression
Target Milestone: 4.2.0Flags: damjan: 4.2.0_release_blocker?
Hardware: All   
OS: All   
Issue Type: DEFECT Latest Confirmation in: 4.2.0-dev
Developer Difficulty: ---
Attachments:
Description Flags
Step 1
none
Step 2
none
Problem
none
Allow empty selection in PageSelector::UpdateCurrentPage none

Description fanyuzhen 2014-04-25 08:28:53 UTC 
Steps:
1. Create several slides which can activate vertical scrollbar in Slides window, for example, 8 slides in my Win 7 - see step 1.jpg
2. Click slide 6 (slide 6 is selected, scrollbar moves down) - see step2.jpg
3. Put mouse on the blank area between slide 5 and slide 6, then right click

Actual result: see result.jpg
1. Slide 1 is selected incorrectly
2. Slide 5 gets activated incorrectly 
3. Right click menu(New Slide) displays over the slide 5, then the new slide will be inserted between slide 4 and slide 5 wrongly

Expected result: 
1. Slide 6 is selected
2. Draw a black line in the position between slide 5 and slide 6, to show you where the slide will be added
3. Right click menu(New Slide) displays in the blank area between slide 5 and slide 6, the new slide will be inserted between slide 5 and slide 6

Root cause: Whichever the slide is selected, when you click/right click in the Slides window(thumbnail tray), the focus will go to slide 1 (slide 1 is selected) incorrectly.

Note: it's a regression from AOO4.0.1
Comment 1 fanyuzhen 2014-04-25 08:30:24 UTC 
Created attachment 83272 [details]
Step 1
Comment 2 fanyuzhen 2014-04-25 08:30:47 UTC 
Created attachment 83273 [details]
Step 2
Comment 3 fanyuzhen 2014-04-25 08:31:41 UTC 
Created attachment 83274 [details]
Problem
Comment 4 Andre 2014-04-25 09:29:07 UTC 
Could be connected to bug 123197.  Is this still reproducible in the RC4 (or at least RC3)?
Comment 5 Edwin Sharp 2014-04-25 10:28:50 UTC 
Confirmed with
AOO410m17(Build:9763)  -  Rev. 1586584
2014-04-11 09:13 - Linux x86_64
Debian
Comment 6 fanyuzhen 2014-04-25 11:07:45 UTC 
It is very inconvenient and frustrates user, propose to be a show stopper if the risk of fix can be contained.
Comment 7 jsc 2014-04-25 12:06:54 UTC 
I agree that it is not nice but for me it is no showstopper but a normal issue. Yes it is a regression but again it is not serious. We have no data loss and it can be easy workarounded. We even don't know if it is often used. Otherwise I am wondering why we detect it so late. The related code changes were probably made in February and the issue was already in the Beta.

When you click on a slide and insert a new slide via the context menu it works.

I would fix it asap on trunk and take into account if others more serious issues are found.
Comment 8 Andre 2014-04-25 12:38:02 UTC 
Created attachment 83276 [details]
Allow empty selection in PageSelector::UpdateCurrentPage

This is a fix for the problem at hand.  It may, however, be too simple and break something else.
Comment 9 Oliver-Rainer Wittmann 2014-04-25 14:27:41 UTC 
I applied the patch to my local trunk development environment and it worked for me.

Nevertheless, I would not 'stop our 4.1 release show' for this issue as its severity is not high enough from my point of view.
Comment 10 Marcus 2017-05-20 10:45:07 UTC 
Reset the assignee to the default "issues@openoffice.apache.org".
Comment 11 damjan 2023-10-05 19:11:55 UTC 
This is still an issue in the latest trunk.

That PageSelector::UpdateCurrentPage() method that the patch here is for, has only has 2 sources of changes, as shown by "git blame main/sd/source/ui/slidesorter/controller/SlsPageSelector.cxx", the original commit where it was imported by Bob Weir, and 4799f5ba13c by Andre Fischer for bug 123197.

Reverting 4799f5ba13c fixes the issue, therefore this is a regression from that commit:

---snip---
commit 4799f5ba13c0140a13cf2e05fc58d1eb59f25ad2
Author: Andre Fischer <af@...>
Date:   Fri Feb 21 11:55:33 2014 +0000

    123197: Fixed selection problems when switching between normal and master mode.
---snip---



Andre: any ideas?
Comment 12 damjan 2023-10-08 19:09:25 UTC 
That patch works and seems to make sense, so I am going to commit it:

---snip---
commit 2a12cc4fae255902f4870bfea0bfe724d9d4ddb8 (HEAD -> trunk, origin/trunk, origin/HEAD)
Author: Damjan Jovanovic <damjan@...>
Date:   Sun Oct 8 21:01:32 2023 +0200

    Allow empty selection in PageSelector::UpdateCurrentPage().
    This fixes a regression in Impress, where wrong slides are
    selected on a right-click between slides when scrolling down,
    and new slides are inserted in wrong places.
    
    Patch by: Andre Fischer <af@...>
    Reviewed by: me
    Fixes: #124741 - Slide will be inserted in wrong place when try to
      insert it in latter slides(with vertical scrollbar)
---snip---


Resolving FIXED. Thank you for your bug report and patch!
Comment 13 damjan 2023-10-08 19:11:21 UTC 
Cherry-picked for AOO42X with 0377632d55e2f6c71bc9597b5614b3773b09d374.
Comment 14 damjan 2023-10-08 19:12:54 UTC 
Let's not release it in 4.1.15 yet, could use a little more testing.