140275 - drag-selecting over unclosed font tags freezes mozilla

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Torgeir Veimo]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.9+) Gecko/20020424
BuildID:    2002042412

I am able to freeze mozilla completely by just selecting text multiple times
rapidly. It is not easy to reproduce, but happens several times a day for me. 

Reproducible: Sometimes
Steps to Reproduce:
1. select text
2. select text again, drag mouse around a lot
3. selecte text again, freeze

Happens on different machines.

Comment 1

[Adrian Ulrich]

when you select text again and again, you also drag and drop the text?

Can't reproduce this with marking text very fast.. but when i try to do that, i
automaticly drag some text:

Dup of 81779 ? okay?

Comment 2

[Torgeir Veimo]

Going from rc1 to 2002042412 (which supposedly has the fix from bug 81779)
doesn't change anything. It happens mostly on complex pages, eg. dagbladet.no
(norwegian newspaper). 

Comment 3

[Torgeir Veimo]

I just reproduced this and noticed, that it indeed seem to be while dragging,
but it happens both when there is text selected, and when there is no text selected.

Comment 4

[Torgeir Veimo]

Here's a way to reproduce it;

1. Go to the page http://db.no. 
2. Scroll down to the topic "Får ikke si 'palestinskje ofre'"
3. select part of the text to the left of the image
4. press the mouse in the are to the left with the title "blink", eg. just right
of the image contained within
5. drag into the area where there is text selected.
6. complete freeze 

Comment 5

[Torgeir Veimo]

Attachment: screenshot of page for non-norwegian readers describing steps to reproduce

Comment 6

[Torgeir Veimo]

Actually, the page mentioned changes quite often. The "blink" area seem to stay
though. The important thing is to drag from within that area, just right of the
image.

Comment 7

[Torgeir Veimo]

just checked with build id 2002042510, same reproducible result.

Comment 8

[David Evola]

Win2000, Build 2002042510

Test with page http://db.no results in freeze of all Mozilla windows.

Comment 9

[Adrian Ulrich]

Ok, bug 81779 was linux (gtk?) only..

then it has nothing to do with this bug

Comment 10

[Andreas Kunz]

Able to reproduce it with 2002042510 and with RC1 (Win2k): freeze, 99% CPU usage
for minutes. Once there even was no text selected first, I only pressed the
mouse button at the bottom of the blue box on the left (kind of weather
thing...) and moved the mouse cursor to the lower right. While moving, a part of
"Chamonix" was selected, the mouse cursor turned into a vertical bar (text
cursor) and stayed like that - mozilla freezed.
After reproducing it 2 or three times now it seems not to work (=freeze) anymore
for me... strange. 
Perhaps the page has been changed. I thought, the 'palestinskje  part was near
to the blue weather box when I was able to reproduce. Now it isn't anymore...

Comment 11

[Torgeir Veimo]

It would be very helpful it is was possible to get a "thread dump" or something
similar when mozilla crashes, if it was started from the console, and one
pressed a key combination of some sort. Under java, one can press ctrl-\ to get
a stack / thread dump. I use this all the time to find GUI deadlocks in netbeans
development releases.

Comment 12

[Mark Bitterling]

I seem to be unable to reproduce this on OS X Mozilla 1.0 branch build 2002042605 

Comment 13

[Torgeir Veimo]

Attachment: tar file containing a snapshot of the page

Comment 14

[Torgeir Veimo]

To reproduce the bug in the site snapshot; find the left column box with the
title "blink". Select text in the article to the right of this box. Then click
just to the right of the image contained within the blink box and drag.

It might be that the bug is caused by the complexity of the site and that the
drag freezes when crossing the border between some parts of the page.

Anyway, I'll try to remove all unnecessary parts of the page and see when the
problem disappears.

Comment 15

[Andrew Schultz]

Attachment: stacktrace during hang

Comment 16

[Andrew Schultz]

marking NEW
OS=>All

this started happening between build 2002032608 and 2002032712.  also, I did not
need to select any text on the right side before drag-selecting across frames
from the left.  base on the stacktrace, it looks like a frames problem...

==> HTML Frames

Comment 17

[Andrew Schultz]

adding URL, updating summary

ok, so there aren't any frames in the page, it's just tables.  I dunno where
this should be...

regression happened during the time window of the checkin for bug 129350, which
modified nsCSSFrameConstructor.cpp (which is where the hang seems to happens).

cc'ing dbaron.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Very unlikely to be related.  nsCSSFrameConstructor.cpp is a huge file.  This
looks like it might be a malformed frame tree.

Comment 19

[Andrew Schultz]

Attachment: testcase

start at "start here" (either inside or after).  drag select down.
result: nothing happens for a few seconds.  eventually, it will select the rest
of the text.

it appears that it's just *really* slow.  if I delete parts of the page below
"start here", it gets faster even if those parts aren't selected.

also of interest:  there is a comment in the testcase "<!--henvisninger-->". 
if the comment is deleted, it works fine.

Comment 20

[Andrew Schultz]

Attachment: simpler testcase

unbalanced <font> tags seem to also be necessary.  removing (or closing) them
fixes the problem.  the (empty) table at the bottom is also necessary.

Comment 21

[John Keiser (jkeiser)]

Those problems both relate to parser/content sink (though I'm betting on content
sink).

Comment 22

[harishd]

I'm able to reproduce the hang and every time the stack points to 
nsCSSFrameConstructor::FindFrameWithContent(). I'm not sure how this could be a
parser/sink problem. Giving bug to layout gurus for further investigation.

Comment 23

[Laura Krietemeyer]

Changing QA Contact

Comment 24

[Peter Thomassen]

Hi,
you also can reproduce the problem in http://www.assistance4all.de/new/ - simply
select some text from the list in the middle an drag'n'drop it somewhere: crash.

Bye,
Peter

Comment 25

[Peter Thomassen]

Re,
I forgot: there are _no_ font tags and _no_ tables in this document.

Bye,
Peter

Comment 26

[Andrew Schultz]

Peter: you are seeing a different bug (perhaps bug 139629).  this bug is a hang,
not crash, and occurs when selecting text, not drag&drop.

Comment 27

[Peter Thomassen]

Hi,
ok. Where's the difference between "hang" an "crash"?
BTW: The problem on my page also occurs when you drop the selected text part
into the text part itself.

Bye,
Peter

Comment 28

[Jens Müller (:tessarakt)]

See <http://bugzilla.mozilla.org/describekeywords.cgi>, keywords hang and crash.

Comment 29

[Peter Thomassen]

Hi,
thanks.

Bye,
Peter

Comment 30

[Jayesh Sheth]

The attached simpified test case makes Mozilla unresponsive for me every time I
try. I am using
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0rc2) Gecko/20020504

I was able to "unfreeze" Navigator once using Norton Crash Guard, but Mail which
was also open became progressively worse, which resulted in the whole system
crashing.

When I "terminate" Mozilla, I can restart it okay.

Comment 31

[Andrew Schultz]

regression from bug 129350
CVS 2002-03-26 14:35 does not exhibit this bug
CVS 2002-03-26 14:40 does exhibit this bug

==> Style System

Comment 32

[Andrew Schultz]

I meant between 18:35 / 18:40

Comment 33

[Jens Müller (:tessarakt)]

That's
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=03%2F26%2F2002+18%3A35%3A00&maxdate=03%2F26%2F2002+18%3A40%3A00&cvsroot=%2Fcvsroot
then.

So this fix produced a new bug ...

Comment 34

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The problem here is an apparently, based on my stopwatch, O(4^n) (where "n" is
the number of repeated lines in the testcase) algorithm in FindFrameWithContent.

I don't see how my fix would have caused this (although I haven't looked back at
what I did yet).  However, I decided to try and fix this from "first
principles".  So I looked at FindFrameWithContent, and proceeded to "fix"
everything that "looked broken" until the bug went away.  On my fourth or so
try, the bug did go away.  But I think my other changes are good too. :-)

Essentially I fixed it by undoing part of the patch for:
----------------------------
revision 1.394
date: 2000/03/23 23:18:55;  author: nisheeth%netscape.com;  state: Exp;  lines:
+25 -19
r=buster.  bug 31644.  FindPrimaryFrameFor() now accounts for "special" frames
created when blocks are encountered within inlines.
----------------------------
so that we don't recur when we hit |IsFrameSpecial|, which I don't think makes
sense given the current methods for handling special frames.  (IIRC, we always
create a frame hierarchy that corresponds to the parent-child relations in the
content tree by creating an "anonymous" block for each of the inlines in the
ancestor chain of the block within inlines.

I also made some other changes (in the following order, with the change above last):
 * Changed the |GetNextInFlow| to a function that gets the special sibling if
there is no next-in-flow and removed some "don't use the hint if
|IsFrameSpecial|" code.
 * Removed all the funky gotos and turned them into while loops.  There was no
need for gotos.  Also removed the need for the |firstTime| variable by nulling
out |aHint|.  I'm almost wondering if they were used so someone didn't need to
reindent the code.
 * Turned some funky code that double-checked that we weren't about to return an
":after" frame, and, if we were, just silently returned the ":before" frame
instead of the ":after" frame into a real assertion, with no non-DEBUG checking
(i.e., if we hit the assert, I'll be changing the behavior in DEBUG builds to
return the ":after" frame instead of the ":before" frame, which I don't think is
a problem since either would be pretty bad).
 * Fixed code that was modifying |aParentFrame| but I think shouldn't have been.
 * Finally (the fix above), removed the other |IsFrameSpecial| check so that we
don't automatically search the child too (see above).

Two patches to be attached (diff -u, and diff -uw, since I did a lot of
reindenting).

I'm thinking this is a little scary a patch for mozilla1.0, though, so maybe I
should start banging on a reduced version of it for the branch, if we think we
need to fix this on the branch...

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I guess what could have caused this in my patch for bug 129350 was my patch to
ensure that we copied the NS_FRAME_IS_SPECIAL bit to continuing frames when we
split frames, which I thought was the right thing to do, since we marked
continuing frames if they already existing when creating special frames.

Comment 36

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch (diff -u, for testing or checking in)

Comment 37

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch (diff -uw, for review)

Comment 38

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

> * Fixed code that was modifying |aParentFrame| but I think shouldn't have been.

Ditto for modifying |parentContent|, which I therefore removed in favor of
|aParentContent|.

Comment 39

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

... and an assertion that the next-in-flow or special sibling had the same
content. (Is that too silly to assert?  It almost seems like it to me, but the
old code wasn't even assuming it was true.)

Comment 40

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: proposed branch fix (minimal changes to fix bug)

Comment 41

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: proposed branch fix (minimal changes to fix bug)

with the comment changed too, this time...

Comment 42

[Chris Waterson]

Comment on attachment 82411 [details] [diff] [review]
patch (diff -u, for testing or checking in)

sr=waterson

Comment 43

[Chris Waterson]

Comment on attachment 82414 [details] [diff] [review]
proposed branch fix (minimal changes to fix bug)

sr=waterson

Comment 44

[Marc Attinasi]

Comment on attachment 82414 [details] [diff] [review]
proposed branch fix (minimal changes to fix bug)

r=attinasi

Comment 45

[Marc Attinasi]

Comment on attachment 82411 [details] [diff] [review]
patch (diff -u, for testing or checking in)

r=attinasi

Comment 46

[Madhur Bhatia]

nominating for nsbeta1 (fix for rtm)

Comment 47

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk 2002-05-16 06:39 PDT.

Comment 48

[Madhur Bhatia]

cc'ing myself

Comment 49

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This patch caused bug 145224, which has been fixed.

Comment 50

[Torgeir Veimo]

I still see this bug on rc3 on http://dagbladet.no. Will the fix be in 1.0?

Comment 51

[Andrew Schultz]

This was never nominated for 1.0 and it's too late now.

Unless this is wanted for 1.0.1, shouldn't this be marked FIXED?

Comment 52

[Torgeir Veimo]

I don't understand why this isn't 1.0 material. It's a crash, which makes me
loose data if it happens simultaneously with composing mail or editing pages.
This looks bad if it's still present in 1.0.

Comment 53

[Hixie (not reading bugmail)]

We have thousands of crashes, we can't wait til we've fixed every single one of
them before releasing. We'd be here for decades.

Comment 54

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Should be marked fixed since it's fixed on the trunk.

Comment 55

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 82414 [details] [diff] [review]
proposed branch fix (minimal changes to fix bug)

This simple branch fix actually scares me a bit.  I'd rather take the whole fix
that's been on the trunk.

Comment 56

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 57

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to MOZILLA_1_0_BRANCH, 2002-06-20 20:04 PDT.

Comment 58

[Madhur Bhatia]

verified fixed on win20000 build -  2002-07-01-08-1.0 branch 
changing  keyword to verified1.0.1