D10455: add rtl support to textpage
fahadalsaidi updated this revision to Diff 27043. fahadalsaidi edited the summary of this revision. fahadalsaidi added a comment. add autotest for searching in Arabic REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10455?vs=27038&id=27043 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10455 AFFECTED FILES autotests/data/arabic-search-test.pdf autotests/searchtest.cpp core/textpage.cpp To: fahadalsaidi, #okular, aacid, ltoscano Cc: ngraham, michaelweghorn, aacid
D10455: add rtl support to textpage
fahadalsaidi added a comment. @aacid I want to write autotest but there is a bug #390357 in cmake test. I will try make autotest smiliar to test311232() but there is a problem, the pdf file doesn't load probably and it give me in an empty m_generator. please have a look on it. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: ngraham, michaelweghorn, aacid
D10455: add rtl support to textpage
fahadalsaidi updated this revision to Diff 27038. fahadalsaidi edited the summary of this revision. fahadalsaidi added a comment. Well, I was over-optimistic. This patch is related only to BUG: 207748 & BUG:184399. The other back-ends have many problem not related to this patch as following: - xps: okular crash when open arabic xps file and if it can open it, it renders it in wrong way. - dejuv: searching & copying Arabic text are fine since dejuv back-end has it own search function. - odt: there is broken in LTR or RTL. There is no text to search at all. - chm: Although okular can open it but searching is bad for LTR & RTL text, it is basically broken. @aacid the poppler bug should be fixed, so anybody using QT interface can get the right order for RTL langs. Okular has its own way to sort words. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10455?vs=26983&id=27038 REVISION DETAIL https://phabricator.kde.org/D10455 AFFECTED FILES core/textpage.cpp To: fahadalsaidi, #okular, aacid, ltoscano Cc: ngraham, michaelweghorn, aacid
D10455: add rtl support to textpage
aacid added a comment. Thanks for trying to fix this :) Commented code should not be there. this needs an autotest though to make sure we don't break it in the future Are you sure this needs to be done at this level? i.e. have you tried this on xps/chm/djvu/dvi with RTL text to make sure you're not breaking that? Also, in the poppler bug you mentioned this should happen in poppler, and now you're making a patch for okular, will you close the bug in poppler then? Or should this happen in poppler? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: ngraham, michaelweghorn, aacid
D10455: add rtl support to textpage
fahadalsaidi added reviewers: aacid, ltoscano. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 To: fahadalsaidi, #okular, aacid, ltoscano Cc: michaelweghorn, ngraham, aacid
D10455: add rtl support to textpage
fahadalsaidi created this revision. fahadalsaidi added a reviewer: Okular. fahadalsaidi added a project: Okular. fahadalsaidi requested review of this revision. REVISION SUMMARY Right now all text are sorted in LTR order in TextPagePrivate::correctTextOrder() regardless from where they come. Since poppler has fixed it since 0.40, I've just ported reorderTex() from poppler to okular and it works. I am not sure what is unicodeTypeL() & unicodeTypeR() mean in poppler context so I just use isRightToLeft() instead. This patch intended to fix the following bugs: BUG: 207748 BUG: 353299 BUG: 353300 BUG: 353301 BUG: 184399 REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10455 AFFECTED FILES core/textpage.cpp To: fahadalsaidi, #okular Cc: michaelweghorn, ngraham, aacid
