Closed Bug 282760 Opened 20 years ago Closed 19 years ago

Rendering gives Hebrew text OUTSIDE of HTML table cell for some window sizes.

Categories

(Core :: Layout: Text and Fonts, defect)

x86
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: mail, Assigned: uriber)

References

()

Details

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.3) Gecko/20040910

In http://www.cvkimball.com/Tanach/TanachSearch.xml?Gen37:1-37:4, if the 
containing horizontal window size is adjusted about 75% smaller than the 
average user would start the page, eventually (when Zilpah is the last word on 
the third line, Zilpah is the 16-th word in verse 37:2) the Hebrew text on the 
fourth line occurs outside the table.  When this occurs, a horizontal slide bar 
occurs, which if shifted to the right, shows the text outside the table cell.

Reproducible: Always

Steps to Reproduce:
1. Open the referenced web page with a nearly full screen horizontal window 
size. 
2. Gradually reduce the horizontal window size, looking for text outside the 
table border or for the appearance of a horizontal position slider.
3. Observe the text is outside the table.

Actual Results:  
Text is outside the table and may be missed by the user.

Expected Results:  
The text should always be within the table border.

Occurs in all Mozilla browsers, Firefox, Netscape, Mozilla.  Works fine in IE6.
Occurs without the latest MS usp10.dll module.

Text is XSLT converted into the textarea.

I have NOT been able to replicate this with a stand-alone HTML file.

This results in a missing word which is major problem for this public service 
site which is used in Biblical Hebrew classes throughout the world.
Not sure if this is BiDi-specific, but it definitely doesn't belong in firefox:
general...
Assignee: firefox → mkaply
Component: General → Layout: BiDi Hebrew & Arabic
Product: Firefox → Core
QA Contact: general → zach
Version: unspecified → Trunk
Severity: critical → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Please see much-simplified example at:

 http://www.cvkimball.com/MozillaTest/Genesis.DH.xml 

which doesn't use JavaScript or internal XSLT transformations.

To see the problem, reduce the horizontal size of the browser window until text
appears outside the table boundary on the right, as in the original description.
The out-of-cell text can be eliminated by
removing one set of <span> tags and by eliminating non-Hebrew characters
inside spans in which Hebrew font is specified.

1. The original text has an initial <span> tag setting the right-to-left
direction (line 293).  Included later in this span is another span
setting the font to SBL Hebrew and font size (line 343/348).  If the second span
at line 343 is eliminated with the font and font size information being placed
with the first span, the original problem on browser line 4 disappears.

2. For some choices of horizontal width, however, some other text falls outside
the table cells.  If the 00A0's at lines 930 and 935 are placed within the <span>
tag rather than outside, the text remains in the table for all observed horizontal
widths.

A Tanach.xsl.xml file modified as above is in:

http://www.cvkimball.com/MozillaTest/Fixed/Genesis.DH.xml.

Discussion: I'm not aware of any imbedding restrictions on <span> tags.  The second
set of <span> tags occur only 6 times in the example and at one level, so it's
not likely that a counter is being over-run.

*** The file at

http://www.cvkimball.com/Tanach/TanachSearch.xml

will be repaired shortly, please use the simplified example at

http://www.cvkimball.com/MozillaTest/Genesis.DH.xml.

to explore this problem from now on.  ***
Attached file Minimized testcase (obsolete) —
The text should wrap to the second line without overflowing the lime-coloured
background.
Assignee: mkaply → smontagu
Status: NEW → ASSIGNED
Attached file Minimized testcase
Attachment #174962 - Attachment is obsolete: true
The minimized test case doesn't demonstrate the problem.

The problem seems to depend on which characters start the line and on character
size.
Attached patch patch (obsolete) — Splinter Review
I found the the problem is that ComputeWordFragmentDimensions() is passing the
wrong (maximum) word length to tx.GetNextWord(). It's passing the length of
"this" frame, while GetNextWord is actually working on the next frame.
As a result, in a case such as <span>aa</span>bbbbb, GetNextWord() will only
return the first two "b"s (there are two "a"s in this example), which might
lead to the false conclusion that the entire word (aabbbbb) fits in the line,
when actually only aabb does.

So the main thing this patch does is to look at the content offset and length
of the next (text) frame, rather than "this" frame, when calculating a
(maximum) wordLen.

Additionally, I renamed aTextFrame to aNextFrame, to be cosistent with the
method declaration (on line 825). Although it *is* a text frame, I think the
more important information about it is that it is the *next* text frame.

I realize that casting aNextFrame to nsTextFrame* looks somewhat unsafe. It is
guaranteed to be a nsTextFrame, because it is ultimately received from
nsLineLayout::FindNextText(), which can only return a text frame (or null).
Perhaps a more correct solution would be to change the return type of
nsLineLayout::FindNextText() to nsTextFrame*, and have it do the casting?

Another question is do I have to change mState to
((nsTextFrame*)aNextFrame)->mState as well when checking for NS_FRAME_IS_BIDI.
It seems to me that if one frame is NS_FRAME_IS_BIDI than so is the other, but
I might be missing something.

Finally, I want to apologize for working on a bug which is assigned to someone
else (Simon). I only noticed this after I already did a lot of investigagtion
work, so I thought it was worth publishing my resluts anyway. Simon - if my
approach seems OK, and if you don't mind, could you please re-assign this to
me?
Attachment #186014 - Flags: review?(smontagu)
Assignee: smontagu → uriber
Status: ASSIGNED → NEW
Comment on attachment 186014 [details] [diff] [review]
patch

I think it would be better to use aNextFrame->GetStateBits() and
aNextFrame->GetOffsets() rather than accessing its members directly. This also
means you don't have to bother about a cast, because GetOffsets() on a non-text
frame will just return zeros.
Attached patch patch v. 2Splinter Review
Use aNextFrame->GetStateBits() and aNextFrame->GetOffsets() as suggested. I had
to expand the ?: construct into an "if" to avoid calling GetOffsets() when
NS_FRAME_IS_BIDI is off.
Attachment #186014 - Attachment is obsolete: true
Attachment #186367 - Flags: review?(smontagu)
Attachment #186014 - Flags: review?(smontagu)
Status: NEW → ASSIGNED
Attachment #186367 - Flags: review?(smontagu) → review+
Attachment #186367 - Flags: superreview?(roc)
Comment on attachment 186367 [details] [diff] [review]
patch v. 2

nice
Attachment #186367 - Flags: superreview?(roc) → superreview+
Comment on attachment 186367 [details] [diff] [review]
patch v. 2

Any chance of getting this to 1.8? The fix is pretty simple and
straightforward.
Attachment #186367 - Flags: approval1.8b3?
Attachment #186367 - Flags: approval1.8b3? → approval1.8b3+
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Verified fixed with:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050621
Firefox/1.0+
*** Bug 177726 has been marked as a duplicate of this bug. ***
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.

Attachment

General

Created:
Updated:
Size: