fix rtl ltr mixed content reversing #3171
Conversation
There was a problem hiding this comment.
Sorry for the late response. Thanks for this PR!
- Please revert the changes to the files in
dist. They are only updated on release. - Please also revert the changes to package.json/package-lock.json. The bugfix version is already included in the "^" version range (Or is this required for this fix?).
- Please also remove all the new *.ttf files again, since they are not used in the tests.
| @@ -1,5 +1,5 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en"> | |||
| <html lang="en" dir="rtl"> | |||
There was a problem hiding this comment.
I don't think this is necessary. I would prefer leaving it ltr, because the page is in English.
| const rtlLangCharsRegex = /[\u0590-\u05FF\u0621-\u064A]/; | ||
| if (rtlLangCharsRegex.test(text)) { | ||
| return [ | ||
| text | ||
| .split("") | ||
| .reverse() | ||
| .join(""), | ||
| posX, | ||
| posY | ||
| ]; | ||
| } else { | ||
| return [text, posX, posY]; | ||
| } |
There was a problem hiding this comment.
While this change somehow seems to fix the issue, I suggest a different one: If I'm correct, the bidiEngine is just not invoked with the correct parameters. When I set the bidi options to
options.isInputVisual = false;
options.isOutputVisual = true;
options.isInputRtl = false;
options.isOutputRtl = false;
everything seems to work correctly, if I set
doc.setR2L(false);
If I understand correctly, the jsPDF R2L option does not specify the base direction of the text. Instead, it specifies if the logical order of the characters in the string is RTL.
There was a problem hiding this comment.
Is this fix suggestion works for u with hebrew/rtl language fonts?
doc.setR2L(false);
......
......
options.isInputVisual = false;
options.isOutputVisual = true;
options.isInputRtl = false;
options.isOutputRtl = false;
doesn't work for me the numbers are still reversed...
There was a problem hiding this comment.
This works for me:
const doc = new jsPDF();
doc.addFont(
"https://cdn.jsdelivr.net/npm/[email protected]/fonts/Rubik-Regular.ttf",
"Rubik",
"normal"
);
doc.setFont("Rubik"); // set font
doc.setFontSize(50);
doc.setR2L(false);
const mixedRtlLtrText = "שלום עליכם 123";
const options = {
isInputVisual: false,
isOutputVisual: true,
isInputRtl: false,
isOutputRtl: false
};
doc.text(mixedRtlLtrText, 10, 20, options);
doc.text("1234 ", 10, 40, options);
doc.text("1234", 100, 40, options);
doc.text(" שלום עליכם ", 10, 60, options);
doc.text("abc ", 10, 80, options);
doc.text("abc", 100, 80, options);
doc.save();
Is the outcome correct? I'm not a Hebrew/RTL expert...
There was a problem hiding this comment.
Yes, it is correct.
Do you know how to make it work with autotable?
There was a problem hiding this comment.
@liorokach Not sure. We probably need to fix it in the jsPDF library and then it will work without passing custom options to the bidi engine.
@ronbiter Could you make the suggested changes?
There was a problem hiding this comment.
Hey, any updates on this? I installed the forked ronnibitter repo and mixed content shows up correctly, but with autotable it's still mixed up.
Let me know if I can help
There was a problem hiding this comment.
hey i'm sorry I don't have the time currently to go back to this. the bidi engine did work when I tested it a while ago but it wasn't perfect at all situations so I kept with my change with few minor tweaks for different situations that we encountered.
you are all welcome to fork the repo and PR it to the project. I don't know currently when I will have the time fix the PR (it is still failing the tests).
*** this fix was mainly for generating PDF from an html code (using html2canvas) didn't test it for other methods.
Thanks for contributing to jsPDF! Please follow our
Contribution Guidelines
when creating a pull request.
on mixed content of languages (for example Hebrew and English and also numbers) the R2L property would reverse the entire text.
the sentence "שלום עליכם 123" would turn to "שלום עליכם 321".
fixed the reversing of the LTR languages and numbers.