Bug 305443

Summary: Manual re-ordering of window tabs is erratic
Product: [Plasma] Oxygen Reporter: Gilboa Davara <gilboad>
Component: win decoAssignee: Hugo Pereira Da Costa <hugo.pereira.da.costa>
Status: RESOLVED FIXED    
Severity: normal CC: hugo.pereira.da.costa, kwin-bugs-null, travisgevans
Priority: NOR Keywords: regression
Version: unspecified   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 4.9.1
Attachments: Manual reordering movie.
updated patch

Description Gilboa Davara 2012-08-19 16:26:42 UTC
Trying to reorder tabs leads to unpredictable results.
(See steps to reproduce).

Reproducible: Always

Steps to Reproduce:
1. Group multiple windows (1.2.3.4.5...N), where N is window on the far right.
2. Try to move windows N-2 to position N-1.
3. Window N-2 will move to position N instead of N-1.
Comment 1 Gilboa Davara 2012-08-19 16:28:16 UTC
Created attachment 73307 [details]
Manual reordering movie.
Comment 2 Thomas Lübking 2012-08-19 18:46:43 UTC
tab before/behind seems to work (using the rmb buttons) so it's likely a problem in the tab before / behind invocation compared to the visual representation (could be my fault when porting the oxygen deco to the new API)
Comment 3 Gilboa Davara 2012-08-21 13:53:58 UTC
OK, thanks for the update.
Let me know if there's a patch / fix that you want me to test.

- Gilboa
Comment 4 Thomas Lübking 2012-08-23 20:58:13 UTC
See https://git.reviewboard.kde.org/r/106149/
Comment 5 Hugo Pereira Da Costa 2012-08-27 22:08:11 UTC
Created attachment 73516 [details]
updated patch

Tested here and works fine.
It includes the patch proposed by Thomas and mentionned in the review board but extends it to
- make the visualization match the actual positionning
- fix some visualization issues.
Comment 6 Hugo Pereira Da Costa 2012-08-27 22:09:03 UTC
@Gilboa 
Tell me if you get a chance to test the patch from comment #5 (possibly before KDE4.9.1 tagging, by thirsday)
Comment 7 Hugo Pereira Da Costa 2012-08-27 22:19:57 UTC
*** Bug 304998 has been marked as a duplicate of this bug. ***
Comment 8 Thomas Lübking 2012-08-27 22:20:54 UTC
The only relevant additional change are the two swapped tabIndex calls, right? (i don't want to damage my git history even more ;-)
Comment 9 Hugo Pereira Da Costa 2012-08-27 22:33:46 UTC
@Thomas
yes:
-            const int clickedIndex( tabIndexAt( position, false ) );
+            const int clickedIndex( tabIndexAt( position, true ) );

(x2)

So, will you include this in your review request and push ? 
Or do you want me to push full patch and discard your request ? 
I *will* commit the clean-up in any case (either in same patch or after yours)
Comment 10 Thomas Lübking 2012-08-27 22:40:40 UTC
I'll first of all check the change and pass you a feedback.

But it's your deco - commit & push whenever you feel like (ensure to close this bug so i see the push)
Comment 11 Thomas Lübking 2012-08-27 23:37:47 UTC
"Ship It!"

Might be matter of taste - both variants do not feel ideal to me.
Comment 12 Hugo Pereira Da Costa 2012-08-28 07:27:27 UTC
Git commit e9b0b5548eb60affb417683eb23a1655e79114e6 by Hugo Pereira Da Costa.
Committed on 28/08/2012 at 09:26.
Pushed by hpereiradacosta into branch 'master'.

Fixed positioning of tabs when manually reordering with right mouse button
Made 'drop target' animation consistent with where tab is actually dropped
Cleanup code.

M  +19   -28   kwin/clients/oxygen/oxygenclient.cpp

http://commits.kde.org/kde-workspace/e9b0b5548eb60affb417683eb23a1655e79114e6
Comment 13 Hugo Pereira Da Costa 2012-08-28 07:28:14 UTC
Git commit 7e5368f6d02d9d34863cf5f5427141693d237ad4 by Hugo Pereira Da Costa.
Committed on 28/08/2012 at 09:26.
Pushed by hpereiradacosta into branch 'KDE/4.9'.

Fixed positioning of tabs when manually reordering with right mouse button
Made 'drop target' animation consistent with where tab is actually dropped
Cleanup code.

M  +19   -28   kwin/clients/oxygen/oxygenclient.cpp

http://commits.kde.org/kde-workspace/7e5368f6d02d9d34863cf5f5427141693d237ad4
Comment 14 Hugo Pereira Da Costa 2012-08-28 07:28:42 UTC
That fixes it. 
Closing.
Comment 15 Gilboa Davara 2012-09-02 15:46:01 UTC
Confirmed, fixed.
Thanks for the fast fix!

- Gilboa
Comment 16 Gilboa Davara 2012-09-06 06:10:05 UTC
Is the fix included in 4.9.1?
Comment 17 Hugo Pereira Da Costa 2012-09-06 06:23:42 UTC
should be yes
Comment 18 Gilboa Davara 2012-09-12 15:47:56 UTC
Working. Thanks.