Review Request: Fix wrong position calculations in AbstractSelectionStrategy, SelectionStrategy and DragAndDropStrategy

[Commit Hook]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102517/#review6658
-----------------------------------------------------------


This review has been submitted with commit ab38a42b294765e696bb0475ed946f3d7ef7a7f2 by Sebastian Sauer to branch master.

- Commit


On Sept. 2, 2011, 8:03 a.m., Sebastian Sauer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102517/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2011, 8:03 a.m.)
> 
> 
> Review request for Calligra, Marijn Kruisselbrink and Stefan Nikolaus.
> 
> 
> Summary
> -------
> 
> This fixes a crash when selecting cell after creating a chart ( bug https://bugs.kde.org/show_bug.cgi?id=279951 ).
> 
> This patch is rather essential and changes how selections are translated to a range of rows and columns. Before we did;
> 
>     const KoShape* shape = tool()->canvas()->shapeManager()->selection()->firstSelectedShape();
>     const QPointF position = documentPos - (shape ? shape->position() : QPointF(0.0, 0.0));
> 
> and now (with this patch) we do;
> 
>     const QPointF position = documentPos;
> 
> What means we don't try to take the position of the first selected shape into account any longer. That seems to work
> great, fixes bug 279951 and also gets right of a few other asserts (e.g. select C3 and keep the mouse to expand the
> selected range to A1 and go a bit future to have also the header in the selection => assert).
> 
> The push that added the firstSelectedShape() to Tables (back then KSpread) was commit 2afef0a9;
> 
>     Author: Stefan Nikolaus <stefan.nikolaus at kdemail.net>  2008-06-14 12:40:03
>     Committer: Stefan Nikolaus <stefan.nikolaus at kdemail.net>  2008-06-14 12:40:03
>     Parent: 781ac3926d0e6224d4547cb5041041f66c4ecd97 (* New, updated TODO list managed using emacs' org-mode)
> 
>     Cell Tool	Switch to strategies for the mouse interaction.
>     		Drag'n'drop is special cased.
> 
> I tried to compile that revision to check if the bug was present but seems I get interesting compile-errors related
> to undefined QString, KGlobal, etc. Maybe cause 2008 we where building still against Qt3? Or against a moving kdelibs
> (aka the KDE4-port)? Not sure there but also later revisions fail to compile... I didn't went future cause since that
> particular commit *lot* of refactoring happens and it can also be the case that one of the many refactorings like
> e.g. commit 3fcae916 did break it. I wouldn't wonder...
> 
> 
> Diffs
> -----
> 
>   tables/ui/AbstractSelectionStrategy.cpp f5982f9 
>   tables/ui/DragAndDropStrategy.cpp f500b5a 
>   tables/ui/SelectionStrategy.cpp 8844e47 
> 
> Diff: http://git.reviewboard.kde.org/r/102517/diff
> 
> 
> Testing
> -------
> 
> I tested various drag and drop and selection cases and they all still seem to work as expected. Also it fixes bug 279951 .
> 
> 
> Thanks,
> 
> Sebastian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110920/88231412/attachment.htm>