Review Request 113969: Do not assume every items have the same height

[Yichao Yu]

> On Nov. 20, 2013, 5:27 p.m., Christoph Feck wrote:
> > I love people who report bugs, and one year later come up with a patch :P
> > 
> > Anyway, nice analysis, and this probably also fixes bug 290971, but have not tested it yet.
> 
> Yichao Yu wrote:
>     Unfortunately I think I can still reproduce those problems. (I noticed the problem when testing my patch and it's good to know it is not caused by me (not likely anyway...))
>     
>     There are indeed other places in the code that suffers from the same problem (blockHeight and intersectingIndexesWithRect looks suspicious at least) but I am not sure and I think it is a better idea to fix them one by one...
>

Ping. Can someone review this?

I'm not completely sure if this is the best solution for the bug but I believe it is the right place to fix it and I'm willing to improve the patch if someone can provide some suggestions.


- Yichao


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


On Nov. 20, 2013, 4:47 p.m., Yichao Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113969/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2013, 4:47 p.m.)
> 
> 
> Review request for kdelibs, David Faure, Rafael Fernández López, and Michael Pyne.
> 
> 
> Bugs: 309780
>     http://bugs.kde.org/show_bug.cgi?id=309780
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> 1. the offset addjust in `KCategorizedView::indexAt` was a no-op (it operates on a temporary variable and is not needed).
> 2. KCategorizedView::indexAt (effectively) assumes all items has the same height when doing bsearch and therefore failed to handle some cases when the text takes multiple lines as shown in the bug report.
> 
> This patch removes the no-op and add special check for items in the same row on the left (or on the right for RightToLeft layout) in order to determine which way the bsearch should go.
> 
> 
> Diffs
> -----
> 
>   kdeui/itemviews/kcategorizedview.cpp 010bcbc 
> 
> Diff: http://git.reviewboard.kde.org/r/113969/diff/
> 
> 
> Testing
> -------
> 
> Compiles and fixes the problem.
> Tested with systemsettings in the following conditions:
> 1. single row in each category.
> 2. multiple rows in each category.
> 3. scrollbar not at the top.
> 
> 
> Thanks,
> 
> Yichao Yu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20131209/fb6c1be8/attachment.htm>