D10455: Add RTL support for search, copy & paste in pdf

guysoft added a comment. Hey, So I sent a merge request for this, but the issue is now that clazy formatting in the ci/cd of the build requires these too lines be changed to something more readable: // output a left-to-right section for (j = i; j < origTxtOrder.length() && !

D10455: Add RTL support for search, copy & paste in pdf

guysoft added a comment. Done, but I can't test it: https://invent.kde.org/graphics/okular/-/merge_requests/595 REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: sander, guysoft, davidhurka, yaron, okular-devel,

D10455: Add RTL support for search, copy & paste in pdf

sander added a comment. Step 1 would be to open a merge request at https://invent.kde.org/graphics/okular/-/merge_requests . Can you do that? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: sander, guysoft,

D10455: Add RTL support for search, copy & paste in pdf

guysoft added a comment. Hey, Any progress on this? What needs to change in core/textpage.cpp in order for this to work correctly? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: guysoft, davidhurka, yaron,

D10455: Add RTL support for search, copy & paste in pdf

davidhurka added a comment. I read the reported test results as if this doesn’t work correctly. However, without this patch it doesn’t work at all, so that may already be an argument to accept this patch. Text handling and search is IMHO broken in general, and needs to be redone. The

D10455: Add RTL support for search, copy & paste in pdf

yaron added a comment. I'm still experiencing this bug. For example: If I want to find the word Shalom I need to search for molahS in order to find a match. Anything I can help with regarding this patch? REPOSITORY R223 Okular REVISION DETAIL

D10455: Add RTL support for search, copy & paste in pdf

aacid requested changes to this revision. aacid added a comment. This revision now requires changes to proceed. Please move as a Merge Request in https://invent.kde.org/kde/okular We have pre-commit CI and lots of checks including clazy and clang-tidy there so it's a much better place

D10455: Add RTL support for search, copy & paste in pdf

fahadalsaidi added a comment. In D10455#263014 , @ngraham wrote: > @fahadalsaidi, are you able to work on those? Sorry, I don't have knowledge & time to fix the memory leaks. For the other question , I've already answered, here is it:

D10455: Add RTL support for search, copy & paste in pdf

ngraham added a comment. @fahadalsaidi, are you able to work on those? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: okular-devel, chfanzil, ngraham, aacid

D10455: Add RTL support for search, copy & paste in pdf

aacid added a comment. In D10455#262444 , @ngraham wrote: > What's our path forward here? Someone should address the memory leaks and someone should address the questions about the impact of this to other formats other than PDF.

D10455: Add RTL support for search, copy & paste in pdf

ngraham added a comment. Restricted Application added a subscriber: okular-devel. What's our path forward here? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: okular-devel, chfanzil, ngraham, aacid

D10455: Add RTL support for search, copy & paste in pdf

fahadalsaidi added a comment. @chfanzil , thanks for testing and sharing your findings. The patch doesn't touch search logic , it deals only with inner text layer in okular. @aacid sorry to get to you late but Unfortunately I couldn't figure out how to fix the leak, my knowledge in C++

D10455: Add RTL support for search, copy & paste in pdf

chfanzil added a comment. I've tested this patch using 2 PDF files: One in Hebrew (it was downloaded using Wikipedia's Download-as-PDF option) the other in Arabic (it was supplied with the patch). F5729640: Open Source (Hebrew Wikipedia).pdf

D10455: Add RTL support for search, copy & paste in pdf

ngraham added a comment. @fahadalsaidi, I recently migrated an older patch of yours from Reviewboard to Phabricator (D10298 ), but I just closed it in favor of this one. If D10298 is still relevant even with this

D10455: Add RTL support for search, copy & paste in pdf

aacid added a comment. In D10455#209288 , @fahadalsaidi wrote: > @aacid Thanks for the feedback. > > > And you have a memory leak in your function as reported by > > valgrind shows that the leak happen in: > >

D10455: Add RTL support for search, copy & paste in pdf

fahadalsaidi added a comment. @aacid Thanks for the feedback. > I'm still unconvinced the text reversion is happening at the proper level. I really a new to okular's code, so please guide me to the right direction. > And you have a memory leak in your function as reported by

D10455: Add RTL support for search, copy & paste in pdf

fahadalsaidi updated this revision to Diff 27517. fahadalsaidi added a comment. - use QTRY_COMPARE REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10455?vs=27043=27517 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10455 AFFECTED FILES

D10455: Add RTL support for search, copy & paste in pdf

aacid added a comment. In D10455#208549 , @fahadalsaidi wrote: > @aacid any feedback? I'm still unconvinced the text reversion is happening at the proper level. Anyway meanwhile please change the test so that uses qtry_compare

D10455: Add RTL support for search, copy & paste in pdf

fahadalsaidi added a comment. @aacid any feedback? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: ngraham, michaelweghorn, simgunz, aacid

D10455: Add RTL support for search, copy & paste in pdf

fahadalsaidi retitled this revision from "add rtl support to textpage" to "Add RTL support for search, copy & paste in pdf". fahadalsaidi edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid,