Bug 323262 - "Find previous" fails at end of line
Summary: "Find previous" fails at end of line
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 0.17.60
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
Depends on:
Reported: 2013-08-07 17:57 UTC by Jaan Vajakas
Modified: 2013-10-18 14:45 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 4.12.0

lineEnd.pdf referred to in Steps to Reproduce (5.80 KB, application/pdf)
2013-08-07 17:58 UTC, Jaan Vajakas

Note You need to log in before you can comment on or make changes to this bug.
Description Jaan Vajakas 2013-08-07 17:57:09 UTC
When clicking the "Previous" button and the search string is at the end of the line, the search fails. Debugging shows that this is because in the function TextPagePrivate::findTextInternalBackward in textpage.cpp the line

  if ( !comparer( str.midRef(offset, min ), query.midRef( j - min + 1, min ),
                            &resStrLen, &resQueryLen ) )

is buggy: e. g. if line in a  PDF file consists of letters "abc" and the search string is also "abc", then str (the text for the last entity of the line) is "c\n", offset=0, min=2, j=2, so the code cited above compares "c\n" to "bc" and finds no match.

The similar line in findTextInternalForward is also buggy if it is possible for entities to have multi-character texts of other forms than <single character> + <newline>, but I don't know if that can happen.

Reproducible: Always

Steps to Reproduce:
1. Open the attached file lineEnd.pdf.
2. Press F3 and type "abc". Okular highlights the first "abc".
3. Click Next. Okular highlights the next "abc".
4. Click Previous.
Actual Results:  
Okular finds no match (the search box takes a reddish color).

Expected Results:  
Okular should highlight the first "abc".
Comment 1 Jaan Vajakas 2013-08-07 17:58:13 UTC
Created attachment 81599 [details]
lineEnd.pdf referred to in Steps to Reproduce
Comment 2 Albert Astals Cid 2013-08-17 15:03:29 UTC
For completion: Jaan says he is interested in fixing this in bug 323263
Comment 3 Albert Astals Cid 2013-10-18 14:44:19 UTC
Git commit dff8bf1b365f28c6a7b7d97e8ca2a0e579738619 by Albert Astals Cid, on behalf of Jaan Vajakas.
Committed on 18/10/2013 at 14:28.
Pushed by aacid into branch 'master'.

Improve searching code

Also simplified code a bit by removing unnecessary calls to toLower in TextPagePrivate::findTextInternalForward and TextPagePrivate::findTextInternalBackward I also fixed a small bug: the letter capital I with dot above (U+0130) did not match itself in case-insensitive mode on Qt 4.8.4 (U+0130 still does not match lowercase i (U+0069), which can be considered another bug, that I didn't fix (although this behavior conforms to the Unicode case folding rules)).

(I did not implement the Knuth-Morris-Pratt algorithm that I promised in a comment of Bug 323263 because on second thought I find that the win, if any, would probably be negligible except for some very special documents and special query strings.)
Related: bug 323263
REVIEW: 112135

M  +155  -136  core/textpage.cpp
M  +11   -5    core/textpage_p.h
M  +307  -1    tests/searchtest.cpp
M  +1    -0    ui/videowidget.cpp