When entring Hebrew text into a form, Hitting ENTER twice crashes Mozilla

RESOLVED FIXED in mozilla1.0

Status

()

Core
Layout: Text
P1
critical
RESOLVED FIXED
17 years ago
3 years ago

People

(Reporter: Shoshannah Forbes, Assigned: smontagu)

Tracking

({crash})

Trunk
mozilla1.0
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2 rtm], URL)

Attachments

(7 attachments, 5 obsolete attachments)

(Reporter)

Description

17 years ago
From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0)
BuildID:    20010801

An easy way to crash Mozilla 0.93 :
* Go to any Hebrew web site
* Type Hebrew text into a form textarea
* Hit the ENTER key on the keyboard twice

The result:
Mozilla crashes without giving any error message (it just dissapears)

I am using Milestone 0.93 on Windows 2000 Server which has Hebrew installed.

Example URLs:
* http://www.ynet.co.il/home/1,7340,L-875-258,FF.html - a logical page.
Click on "add a message" to get the form
* http://www.fisheye.co.il/story.php3?id=514 - a visual page. Click on "add
reply" to get the form.



Reproducible: Always
Steps to Reproduce:
1. Go to http://www.ynet.co.il/home/1,7340,L-875-258,FF.html 
2. Click on the "add messge" button
3. Type Hebrew text into a form textarea
4. Hit the ENTER key on the keyboard twice


Actual Results:  Mozilla crashes without giving any error message (it just 
dissapears)

Expected Results:  Enter two CRLF into the form

This happens both on logical and visual pages, as seen from the URLs above
(Reporter)

Comment 1

17 years ago
Talkback Incident IDs for this bug:
TB34085776H
TB34082393Z
(Assignee)

Comment 2

17 years ago
I can't reproduce this on NT or 2000. Can someone with access to talkback data
attach them to the bug?
Keywords: crash
(Reporter)

Comment 3

17 years ago
Tried again to repro- Didn't work when the messge was Hebrew only, but when I 
wrote Hebrew and then Egnlish, hit Enter twice, I got a crash again.
Talkback ID is:
TB34095309E

I saved the info from that talkback as a file- I am trying to attach it here
(Reporter)

Comment 4

17 years ago
Created attachment 45793 [details]
last talkback info

Comment 5

17 years ago
Here is the stack from the most recent talkback ID above:

0x02ff55a1 
nsCaret::SetupDrawingFrameAndOffset [d:\builds\seamonkey\mozilla\layout\base\src\
nsCaret.cpp, line 570] 
nsCaret::DrawCaret [d:\builds\seamonkey\mozilla\layout\base\src\nsCaret.cpp, line 
898] 
nsCaret::StartBlinking [d:\builds\seamonkey\mozilla\layout\base\src\nsCaret.cpp, 
line 456] 
nsCaret::SetCaretVisible [d:\builds\seamonkey\mozilla\layout\base\src\
nsCaret.cpp, line 210] 
PresShell::SetCaretEnabled [d:\builds\seamonkey\mozilla\layout\html\base\src\
nsPresShell.cpp, line 2996] 
PresShellViewEventListener::RestoreCaretVisibility [d:\builds\seamonkey\mozilla\
layout\html\base\src\nsPresShell.cpp, line 6757] 
PresShellViewEventListener::DidRefreshRegion [d:\builds\seamonkey\mozilla\layout\
html\base\src\nsPresShell.cpp, line 6792] 
nsViewManager::Refresh [d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, 
line 938] 
nsViewManager::DispatchEvent [d:\builds\seamonkey\mozilla\view\src\
nsViewManager.cpp, line 1960] 
HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 68] 
nsWindow::DispatchEvent [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 725] 
nsWindow::DispatchWindowEvent [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 747] 
nsWindow::OnPaint [d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, 
line 4051] 
nsWindow::ProcessMessage [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 3038] 
nsWindow::WindowProc [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 990] 
USER32.DLL + 0x2e98 (0x77e12e98) 
USER32.DLL + 0x39a3 (0x77e139a3) 
USER32.DLL + 0x395f (0x77e1395f) 
ntdll.dll + 0x2032f (0x77fa032f) 
nsEditor::EndPlaceHolderTransaction [d:\builds\seamonkey\mozilla\editor\base\
nsEditor.cpp, line 715] 
nsPlaintextEditor::TypedText [d:\builds\seamonkey\mozilla\editor\base\
nsPlaintextEditor.cpp, line 550] 
nsPlaintextEditor::HandleKeyPress [d:\builds\seamonkey\mozilla\editor\base\
nsPlaintextEditor.cpp, line 524] 
nsTextEditorKeyListener::KeyPress [d:\builds\seamonkey\mozilla\editor\base\
nsEditorEventListeners.cpp, line 264] 
nsEventListenerManager::HandleEvent [d:\builds\seamonkey\mozilla\content\events\
src\nsEventListenerManager.cpp, line 1598] 
nsGenericElement::HandleDOMEvent [d:\builds\seamonkey\mozilla\content\base\src\
nsGenericElement.cpp, line 1675] 
nsHTMLTextAreaElement::HandleDOMEvent [d:\builds\seamonkey\mozilla\content\html\
content\src\nsHTMLTextAreaElement.cpp, line 588] 
PresShell::HandleEventInternal [d:\builds\seamonkey\mozilla\layout\html\base\src\
nsPresShell.cpp, line 5632] 
PresShell::HandleEvent [d:\builds\seamonkey\mozilla\layout\html\base\src\
nsPresShell.cpp, line 5559] 
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 377] 
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 350] 
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 350] 
nsViewManager::DispatchEvent [d:\builds\seamonkey\mozilla\view\src\
nsViewManager.cpp, line 2058] 
HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 68] 
nsWindow::DispatchEvent [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 725] 
nsWindow::DispatchWindowEvent [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 742] 
nsWindow::DispatchKeyEvent [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 2391] 
nsWindow::OnChar [d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, 
line 2516] 
nsWindow::ProcessMessage [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 3510] 
nsWindow::WindowProc [d:\builds\seamonkey\mozilla\widget\src\windows\
nsWindow.cpp, line 990] 
USER32.DLL + 0x2e98 (0x77e12e98) 
USER32.DLL + 0x30e0 (0x77e130e0) 
USER32.DLL + 0x5824 (0x77e15824) 
nsAppShellService::Run [d:\builds\seamonkey\mozilla\xpfe\appshell\src\
nsAppShellService.cpp, line 425] 
main1 [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1297] 
main [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1602] 
WinMain [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1620] 
WinMainCRTStartup() 
KERNEL32.DLL + 0x17d08 (0x77e97d08)
(Assignee)

Comment 6

17 years ago
I have managed to reproduce this occasionally, but not reliably enough to debug
it. Shoshannah, can you send me a sequence of keystrokes which infallibly causes
the crash?

Comment 7

17 years ago
My guess is theFrame is garbage at
569                    theFrame->GetOffsets(start, end);
does that seem likely?
(Assignee)

Comment 8

17 years ago
Could this be related to bug 96604 and/or bug 96841?

(Assignee)

Comment 9

17 years ago
Shoshanna, can you still reproduce this bug now that those other textarea
crashes have been fixed?
(Reporter)

Comment 10

17 years ago
i don't have access anymore to the same machine where I orginally filed the bug on.
On a diffent w2k machine (this time with no service pack while the original had
SP2), I got 1 freeze and 1 crash, but I don't have a clear repro scanrio.
Since that machine has no Internet connection, I am attaching here the deatiled
saved from the talkback.
Is there any better way to get crash info?

I'll be trying to create a more clear scanrio for the crash.
(Reporter)

Comment 11

17 years ago
Created attachment 48009 [details]
Saved talkback info from the last crash
(Reporter)

Comment 12

17 years ago
Arrghh- hate replaying to mayself.
Anyway, forgot to mention that this last test was with build 2001083103.
Sorry for the extra mail :-)
(Reporter)

Comment 13

17 years ago
This bug is starting to annoy  me...
Just go a crash on win98 (Hebrew enabled, 1st edition, Mozilla 2001083103).
I wrote two paragphs in a textarea at http://www.fisheye.co.il , when I
attempted to move to the next paragraph. Hitting Enter didn't seem to move the
crouser, so I hit Enter again a few times, and resoulted in a crash.
Talkback ID is TB3487230W
BTW, I didn't use English at all in that textarea.

On the other hand, I am unable to get a consistent crash :( I do not get any
crashes in english language forms.

/me going back to Fisheye to reconstroct the contents of the messege she lost :-(
(Reporter)

Comment 14

17 years ago
Opps... wrong talkback ID.
The Correct one is:
TB34880978G
Sorry about that...

Comment 15

17 years ago
Here's the trace stack from Shosh's recent crash:
(Mostly identical to the one above.)

Incident ID 34880978
Stack Signature nsSelection::GetFrameForNodeOffset d7ca2712
Bug ID
Trigger Time 2001-09-03 08:29:34
Email Address xslf@yahoo.com
User Comments Typing a form in Hebrew
Build ID 2001083109
Product ID MozillaTrunk
Platform ID Win32
Trigger Reason Access violation
Stack Trace
nsSelection::GetFrameForNodeOffset
[d:\builds\seamonkey\mozilla\content\base\src\nsSelection.cpp, line 2935]
nsCaret::SetupDrawingFrameAndOffset
[d:\builds\seamonkey\mozilla\layout\base\src\nsCaret.cpp, line 570]
nsCaret::DrawCaret [d:\builds\seamonkey\mozilla\layout\base\src\nsCaret.cpp,
line 898]
nsCaret::StartBlinking [d:\builds\seamonkey\mozilla\layout\base\src\nsCaret.cpp,
line 456]
nsCaret::SetCaretVisible
[d:\builds\seamonkey\mozilla\layout\base\src\nsCaret.cpp, line 210]
PresShell::SetCaretEnabled
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 3003]
PresShellViewEventListener::RestoreCaretVisibility
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6788]
PresShellViewEventListener::DidRefreshRegion
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6843]
nsViewManager::Refresh [d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp,
line 936]
nsViewManager::DispatchEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 1960]
HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 68]
nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 732]
nsWindow::DispatchWindowEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 754]
nsWindow::OnPaint [d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp,
line 4071]
nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 3054]
nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 997]
KERNEL32.DLL + 0x363b (0xbff7363b)
KERNEL32.DLL + 0x242eb (0xbff942eb) 
(Reporter)

Comment 16

17 years ago
Not sure if this is related or not:
win98 Hebrew enabled, Mozilla 2001090408
http://www.haayal.co.il/reply.php3?id=681&rep=34140&LastView=2001-09-05%2017%3A06%3A00
(a logical page)
I wrote about 2-3 paragraphs in Heberew, until the textarea was verticly full.
Hit ENTER to move to a new paragraph (and create a vertical scroll bar at the
textarea)- nothing happened. Hit enter again, and got a crash, talkback ID#
TB34971995Q
(note to self- when writing long posts in Hebrew, write them first in notepad.
It is annoying to loose them in a crash...)

I haven't had any crashes in English forms.
I wonder if this is realted to bug 86262 or bug 93958?

Comment 17

17 years ago
marking NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 18

17 years ago
So anyone come up with exact steps to reproduce this? If so, I can probably help 
look into this.

FYI, I fixed a couple of cases where the editor delete code caused the 
textarea's caret to be placed in the browser content, which caused some strange 
crashes.

I mention this because I know that the BIDI code does some selection/caret 
manipulation ... perhaps that is something worth looking into?

Comment 19

17 years ago
Another talkback ID: TB35497270Q

Comment 20

17 years ago
See TB35901135E .

Note: I mentioned there "strange behaviour. I later learned that this is caused
by the fact that the font "Verdana" was used (platform: win98se/hebrew enabled
w/office97 fonts).

See, for instance, http://www.technion.ac.il/~tzafrir/form_hebrew_fonts.html .
Notice the placement of the '.' between the two hebrew words.

MSIE5 uses a different font there instead of Verdana.

This will probably have no effect on other platforms

Comment 21

16 years ago
still happens with 20011005 win32 nightly build on win98se hebrew (enabled).

Currently it seems that every time I edit a long enough text in a textarea with
mixed hebrew/english text (either a UTF-8 or a ISO-8889-8-i page) I get a crash.
Can't think of a better way to reproduce

Comment 22

16 years ago
I believe I have a reproducable sample:
http://www.technion.ac.il/~tzafrir/form_hebrew_fonts.html#problem1
(and follow the instructions from there)

causes mozilla to hang and take 100% CPU (but not to crash). Works both with
0.9.4 and with build 2001100503 (on win98/hebrew)

Updated

16 years ago
Blocks: 115711
(Reporter)

Comment 23

16 years ago
I am getting this with Mac OS as well, build id  2002012903. Changing platform
and OS to "all"
OS: Windows 2000 → All
Hardware: PC → All
(Reporter)

Comment 24

16 years ago
got 2 more crashes on mac 9.2.2.
Talkback IDs:
TB2767184g
tb2667064w
(Reporter)

Comment 25

16 years ago
is any work being done here? The combaination of this bug and the lack of
copy/paste in  the mac build (bugs  #119877 and #119899) make it virtually
impossible to participate in meesage boards using mozilla. Yet another TB crash:
 tb3328264g
(Assignee)

Comment 26

16 years ago
Nominating nsbeta1 per comment 25
Assignee: mkaply → smontagu
Keywords: nsbeta1

Comment 27

16 years ago
crash bug. nsbeta1+ reassign to ftang
Assignee: smontagu → ftang
Keywords: nsbeta1 → nsbeta1+

Updated

16 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0

Comment 28

16 years ago
could someone give me an exact url which have hebrew text area. I don't read
Hebrew so I cannot tell which link are link to "add a message" or "add reply".
Thanks

Comment 29

16 years ago
Created attachment 73799 [details]
Minimal testcase - hebrew form

I have attached a hebrew form to test this bug.
To recreate the bug you must type a combination of hebrew and english text that
would be wrapped around the text entry box and then try to type enter.

Comment 30

16 years ago
crash, so this is a p1
Priority: -- → P1

Comment 31

16 years ago
simon- I can reproduce with the attach test case with HANG now- not crash
Looks like some thing are confused with the the caret position. 
It looks like a bug in layout with bidi but not a keyboard bug . 

could you take a look at this ? I can help you to reproduce it if you cannot.
Assignee: ftang → smontagu
Status: ASSIGNED → NEW
(Reporter)

Comment 32

16 years ago
build 2002031708/ mac os 9.2: I get less crashes for this, but I still get some :-(
Latest talkback ID:
TB4167091E

Comment 33

16 years ago
ok, I can easily reproduce the hang in the following way
1. on WinXP, install hebrew keyboard
2. view the attached form
http://bugzilla.mozilla.org/attachment.cgi?id=73799&action=view
3. in hebrew keyboard, type 'a' 10 times (which produce 10 hebrew) and Shift 'a'
time times (which produce 10 english A), repeate this 3 times without pressing
space, the mozilla will wrap the line 
4. now move the cursor to the first line between two English "A" and press
return, it now hang

Comment 34

16 years ago
Created attachment 75634 [details]
put cursor between 2 "A" in the frist line and hit return, it will hang

Comment 35

16 years ago
Created attachment 75635 [details]
put cursor between 2 "A" in the frist line and hit return, it will hang
Attachment #75634 - Attachment is obsolete: true

Comment 36

16 years ago
hang and crash, adt1
smontagu, please mark as assign, if you agree to work on it.
Whiteboard: adt1
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED

Comment 37

16 years ago
smontagu- any estimation when can we fix this one ?
please put the date on the status whiteboard

Comment 38

16 years ago
Changing impact to ADT2, because Hebrew is not a Tier 1 language for the MachV
release.
Whiteboard: adt1 → [adt2]

Comment 39

16 years ago
 Impact Platform: ALL
Impact language users: Arabic and Hebrew . total 6.3 M 1.125% of total internet
users
Probability of hitting the problem: HIGH, editing any text area in html form may
hit this problem.
Severity if hit the problem in the worst case: hang or crash
Way of recover after hit the problem: kill the app or reboot the machine
Risk of the fix: unknown
Potential benefit of fix this problem: unknown
(Assignee)

Comment 40

16 years ago
There are clearly two different scenarios here.

Bug 136552 has a detailed reproduction procedure which leads to a crash very
similar to the stack traces in comment 5 and comment 15 (compare attachment 78585 [details])

Following the procedure in comment 33 leads to a loop with the same assertions
as in http://bugzilla.mozilla.org/show_bug.cgi?id=129847#c2
Depends on: 129847, 136552
(Assignee)

Comment 41

16 years ago
Created attachment 78771 [details] [diff] [review]
Patch for the crash
(Assignee)

Comment 42

16 years ago
Lina, as the original author I think you are the best person to give r=. Can you
look at attachment 78771 [details] [diff] [review] and tell me if you think it is the correct fix, or
wallpaper over another problem?

Comment 43

16 years ago
r=lkemmel@il.ibm.com

However, there is one point that seems strange to me.
What happened before Simon's patch was deletion of a non-text frame (BRFrame in 
this particular case) -- unnecessary and maybe harmful thing, but not the one 
that should cause a crash. The crash happens because after the frame removing 
the corresponding content node still remains associated with the deleted frame, 
and 
    mTracker->GetPrimaryFrameFor(theNode, aReturnFrame)
in nsSelection::GetFrameForNodeOffset, returns the frame deleted..

Did I miss anything? Is this behaviour intentional?
(Assignee)

Comment 44

16 years ago
What I saw when debugging was that the problem was caused when an |nsTextFrame|
was deleted. Would it perhaps be better to test for the frame type and only
delete |nsContinuingTextFrame|s? I am assuming that an |nsContinuingTextFrame|
is never mapped to content in the frame manager.

Comment 45

16 years ago
latest nightly don't crash on me, or atleast - after testing today several pages
where I always had crashes, I'm willing to submit "works for me". the hang
problem decribed in Tzafrir's page, however, is still there (
http://www.technion.ac.il/~tzafrir/form_hebrew_fonts.html ).

Comment 46

16 years ago
Created attachment 79417 [details]
Talkback info of latest crash

Comment 47

16 years ago
Retraction: and then it crashes. sorry if I bothered anyone. I attached the
crash info. (http://bugzilla.mozilla.org/attachment.cgi?id=79417&action=view)

Updated

16 years ago
Attachment #79417 - Attachment mime type: application/octet-stream → text/plain

Comment 48

16 years ago
are we still crashing ?
IF NO, do we still crash on branch build?
IF Yes, smontagu: should you ask sr= ?
(Assignee)

Comment 49

16 years ago
The crash continues in both trunk and branch, using the procedure described in
bug 136552. I have requested sr= from waterson.

Comment 50

16 years ago
Comment on attachment 78771 [details] [diff] [review]
Patch for the crash

sr=waterson (but avoid double-negative in comment; `only delete bidi frames')
Attachment #78771 - Flags: superreview+
(Assignee)

Comment 51

16 years ago
Created attachment 80436 [details] [diff] [review]
Removed double negative per comment 50
Attachment #78771 - Attachment is obsolete: true
(Assignee)

Comment 52

16 years ago
Comment on attachment 80436 [details] [diff] [review]
Removed double negative per comment 50

Transferring r/sr status
Attachment #80436 - Flags: superreview+
Attachment #80436 - Flags: review+
(Assignee)

Comment 53

16 years ago
Attachment 80436 [details] [diff] checked in on trunk. This fixes the crashing case that I was
able to reproduce (from bug 136552), but leaves the hanging case open. Unless I
obtain some evidence to the contrary, I am assuming that the hanging case is a
dupe of bug 129847 and working on it there. The trunk/branch checkin process is
complicated enough without having more than one issue per bug :-)
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Keywords: adt1.0.0
Resolution: --- → FIXED
Comment on attachment 80436 [details] [diff] [review]
Removed double negative per comment 50

a=shaver for 1.0 branch.
Attachment #80436 - Flags: approval+

Comment 55

16 years ago
adding adt1.0.0-. Let's get this in for RTM.
Keywords: adt1.0.0 → adt1.0.0-
Whiteboard: [adt2] → [adt2 rtm]

Comment 56

16 years ago
Still happened in 2002042221. Not 100% reporoducible, but still did happen. It
was a hang not a crash, with multi-line Hebrew+English text.

Shall I reopen?
(Assignee)

Comment 57

16 years ago
See comment 53. Please only reopen this bug for a crash, not a hang.
(Assignee)

Comment 58

16 years ago
Is there anyone who can confirm or deny that the crash no longer happens? Please
specify build ID and platform tested on.
Keywords: qawanted

Comment 59

16 years ago
I still hang on both win98 and win2k build ID 2002042510. Hang was achieved
using testcase submitted on 3/22/02.
(Reporter)

Comment 60

16 years ago
build id 2002041508 on mac 9.2.2 : this time I got a hang. at :
http://www.tapuz.co.il/tapuzforum/main/addmsg.asp?id=156&msgid=3877240 I wrote
some hebrew text, hit enter , hit the "add code" link (which addes some english
code), and then when looking for the insertion point to move to the new line (i
 hit a few times enter), i got a hang. 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

16 years ago
QA Contact: giladehven → zach
(Assignee)

Comment 61

16 years ago
Read my lips: this bug is no longer about hanging. Please discuss hanging in bug
129847.

If anyone still *crashes* with the test case described in bug 136552 or a
similar scenario, please comment in this bug. If anyone can't reproduce that
crash with a build since 2002-04-23, then please comment _loudly_ in this bug.
Thank you.
QA Contact: zach → giladehven

Comment 62

16 years ago
Can no longer bring it to crash on Linux Mozilla 1.0RC1.
(Assignee)

Comment 63

16 years ago
Sorry, I should have specified "a trunk build since 2002-04-23". The fix is not
in the branch, and certainly not in RC1

Updated

16 years ago
QA Contact: giladehven → zach
(Reporter)

Comment 64

16 years ago
RC1 on win98:
In a text area, I wrote an English word, and then about 3 Hebrew lines. Then I 
went back to the English word, and attempted to select it and type a new word 
on it- got a freeze and had to quit Mozilla from the task manager.

Comment 65

16 years ago
If we want to land this crash case into the branch, we probably need to solve
the case of hang first. Otherwise, there are no way can verify we really fix the
case of crash because it is possible tester hang before they can hit the case
crash. Also, it is better to have crash than hang since we will get talkback
report about crash but no talkback report about hang. hang is worst than crash
since we won't know how many poeple hit that issue. 

smontagu- please continue to work on the case of hang. I don't think we can
really verify THIS bug untill we fix that one. 
(Assignee)

Comment 66

16 years ago
OK, the hang happens (at least in the test case in attachment 75635 [details]) because
after adding the line break we are not going through Bidi resolution again when
reflowing.
(Assignee)

Comment 67

16 years ago
Created attachment 81797 [details] [diff] [review]
Patch for the hang
Adding to RC2 Not Suck bug.  We really want this in 1.0 or mozilla1.0 will be
hard/impossible to use for BiDi locales
Blocks: 138000
(Assignee)

Comment 69

16 years ago
Created attachment 81953 [details] [diff] [review]
Better patch for the hang
Attachment #81797 - Attachment is obsolete: true
(Assignee)

Comment 70

16 years ago
Compare
http://lxr.mozilla.org/mozilla/source/layout/xul/base/src/nsBoxToBlockAdaptor.cpp#1021
Our problem was exactly what is described there: "blocks sometimes send resizes
even when its children are dirty!" Adding the check of NS_FRAME_IS_DIRTY and
NS_FRAME_HAS_DIRTY_CHILDREN will make us correctly do Bidi resolution in these
cases and prevent the hang.
Comment on attachment 81953 [details] [diff] [review]
Better patch for the hang

r=jkeiser
Perhaps a short comment is in order as to what is going on here.  Like "// If
text content has changed, the BIDI direction of some text may have changed"
Attachment #81953 - Flags: review+
(Assignee)

Comment 72

16 years ago
Created attachment 81963 [details] [diff] [review]
Attachment 81953 [details] [diff] plus comment
Attachment #81953 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #81963 - Flags: review+
(Assignee)

Updated

16 years ago
Attachment #81953 - Flags: review+
(Assignee)

Comment 73

16 years ago
I should add that I retract what I said in comment 53: the hang here is not the
same as the hang in bug 129847.

Comment 74

16 years ago
waterson, attinasi, can one of you give this BiDi topcrash fix a super-review?
We're short on time and really need to get this into RC2 (landed before tomorrow).

Comment 75

16 years ago
Comment on attachment 81963 [details] [diff] [review]
Attachment 81953 [details] [diff] plus comment

sr=attinasi, though I would prefer if the comment came _after_ the new line of
code, or before the whole test, slightly reworded to make sense in a slightly
different context.
Attachment #81963 - Flags: superreview+
(Assignee)

Comment 76

16 years ago
Created attachment 82088 [details] [diff] [review]
Modified comment position and wording per comment 75
Attachment #81963 - Attachment is obsolete: true
(Assignee)

Comment 77

16 years ago
Renominating for adt1.0.0, since there is now a fix for both the trunk and the
crash case, and drivers want this for rc2.
Keywords: adt1.0.0- → adt1.0.0
(Assignee)

Comment 78

16 years ago
Fix checked into trunk. In my previous comment, read "there is now a fix for
both the crash and the hang case"
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
(Assignee)

Updated

16 years ago
Attachment #82088 - Flags: superreview+
Attachment #82088 - Flags: review+
(Assignee)

Updated

16 years ago
Attachment #81963 - Flags: superreview+
Attachment #81963 - Flags: review+

Comment 79

16 years ago
Comment on attachment 82088 [details] [diff] [review]
Modified comment position and wording per comment 75

a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #82088 - Flags: approval+

Comment 80

16 years ago
the crash and hang combination in the text area make Mozilla 100% unusable with
arabic and hebrew users . 
we need to following test under the current shor-of-reaction time:
1. all the smoke test to make sure this patch won't cause any layout regression 
2. Look at the trunk tinderbox about the page load. make sure it does not crash
/ hang wiht the page load, neither cause any slow down
3. run the intl page load which they don't run on the trunk tinderbox. make sure
no crash / hang or different layout. 
4. test text area and composer editing with bidi text. 

Comment 81

16 years ago
this is a easy to crash and hang bug with the user group. 

waterson and attinasi : can you measure the risk for this two patches? how risky
it is ? 
(Assignee)

Comment 82

16 years ago
1. Smoketests pass
2. Tinderboxen had no problems running the pageload tests. I don't see any
regressions in the graphs
4. I have been writing in Hebrew with the patch all afternoon without
encountering hangs or crashes.

Looking at intl pageload tests now
(Assignee)

Comment 83

16 years ago
intl pageload tests look OK too.
I'm not waterson or attinasi, but I've spent a lot of hours in this code recently.

Risk on this patch is quite low, and only on pages with BIDI text or textareas.
 The textarea code is  in a BIDI-only routine, and the blockframe code is the
"if ()" leading to a block of BIDI-only code.  If the blockframe is not
BIDIenabled, or if BIDI is not compiled in, it has no effect.  Worst-case
problem is slight perf hit for over-dirtying lines even if things were totally
wrong (and it would still have to be bidienabled to happen).

Low risk, get it in.

Comment 85

16 years ago
adt1.0.0+ (on ADT's behalf) approval for checkin to the 1.0 branch. Pls check
this one in tonight, then add the fixed1.0.0 keyword.
Keywords: adt1.0.0 → adt1.0.0+
Whiteboard: [adt2 rtm] → [adt2]
(Assignee)

Updated

16 years ago
Keywords: adt1.0.0+ → adt1.0.0, fixed1.0.0
Whiteboard: [adt2] → [adt2 rtm]
(Assignee)

Comment 86

16 years ago
Checked both patches into branch

Updated

16 years ago
Keywords: adt1.0.0 → adt1.0.0+

Comment 87

16 years ago
I want more verification be done on the trunk and branch today . simon, can you
send mail out and ask help?

Comment 88

13 years ago
There are a few problems with the attachment 80436 [details] [diff] [review]:

1) A frame marked as a candidate for removal, but actually not deleted,
does not pass Bidi resolution.
(Easy to fix, I guess).

2) Moreover, if the unhandled frame is an |nsContinuingTextFrame|, content 
dupes may occur.
Can we always remove |nsContinuingTextFrame|s, even though are not Bidi?

Updated

10 years ago
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.