Closed Bug 177148 Opened 18 years ago Closed 16 years ago
Presence of hebrew character changes layout
The only difference between these two test cases is that the second contains a hebrew character: http://www.hixie.ch/tests/adhoc/css/text/white-space/normal/line-breaking/bidi/001.html http://www.hixie.ch/tests/adhoc/css/text/white-space/normal/line-breaking/bidi/002.html They render differently. Note: they only render differently because of another bug, so if this bug is not fixed before the other one, new test cases will have to be made.
reassigning per smontagu
Assignee: mkaply → smontagu
This testcase renders incorrectly. It has no hebrew characters. If I add one, it renders correctly. http://www.hixie.ch/tests/adhoc/unicode/bidi/001.html
This patch would fix this bug, but perhaps it is WONTFIX per http://www.unicode.org/unicode/reports/tr20/#Bidi ?
The problem with using markup to do this is that sometimes there is no markup which matches what you are doing, for example in this test case: http://www.hixie.ch/tests/adhoc/css/box/inline/bidi/001.html Does the use of the 'direction' and 'unicode-embed' properties trigger bidi too?
direction: rtl certainly does, and so does any HTML element with dir="rtl". I'm not sure about unicode-bidi: embed. Testcases coming.
Target Milestone: --- → mozilla1.3alpha
Another interesting question is: at what point does the overhead of making all these tests for content that needs bidi processing become higher than just doing bidi processing for all documents? The ICU implementation of the Bidi algorithm which we use is optimized to bail out very soon when all content has the same directionality.
See also bug 288453, which is probably a special case of this and so can probably be marked as a duplicate - but see its test cases. The problem there is that the Unicode characters RLM, RLE and RLO are not taken as content triggering bidi processing - although their entire intention is to control bidi processing. Concerning comment #6, there must be a serious inefficiency if the system needs to check an entire document, potentially megabytes, for bidi trigger characters before it can even start to lay it out.
(In reply to comment #7) > Concerning comment #6, there must be a serious inefficiency if the system needs > to check an entire document, potentially megabytes, for bidi trigger characters > before it can even start to lay it out. That doesn't follow. We start to lay out the document on the assumption that it doesn't require bidi processing. If an RTL character appears megabytes later, it would trigger a reflow. I don't think this would make a huge difference to the overall layout time, but it's worth experimenting with, especially since it is not so unlikely a scenario: many documents (e.g. Wikipedia pages) have a list at the end of other languages in which the document is available, often under their own native names, e.g. Français, Deutsch, עברית, 中文.
I've decided that it's best to just fix this. The patch is the same as before against current trunk.
(In reply to comment #9) > Created an attachment (id=179702)  > Updated patch > > I've decided that it's best to just fix this. The patch is the same as before > against current trunk. Thank you, Simon. But while you are looking at this, is there a good reason for apparently excluding U+07xx and U+08xx from the test? These are also RTL scripts or reserved for them. Or is this somewhere else in the code, not in the patch?
(In reply to comment #10) > But while you are looking at this, is there a good reason for > apparently excluding U+07xx and U+08xx from the test? I made that change on the last cycle, at your suggestion. See http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsBidiUtils.h&rev=1.10&mark=225-242#225
... > > I made that change on the last cycle, at your suggestion. See > http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsBidiUtils.h&rev=1.10&mark=225-242#225 > Thanks! Your memory is better than mine.
*** Bug 288453 has been marked as a duplicate of this bug. ***
Comment on attachment 179702 [details] [diff] [review] Updated patch a=asa
Attachment #179702 - Flags: approval1.8b2? → approval1.8b2+
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.