drag-selecting over unclosed font tags freezes mozilla

VERIFIED FIXED in mozilla1.0.1

Status

()

Core
CSS Parsing and Computation
P1
critical
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Torgeir Veimo, Assigned: dbaron)

Tracking

({hang, regression})

Trunk
mozilla1.0.1
x86
All
hang, regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(6 attachments, 3 obsolete attachments)

(Reporter)

Description

16 years ago
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

16 years ago
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?
(Reporter)

Comment 2

16 years ago
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). 
(Reporter)

Comment 3

16 years ago
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.
(Reporter)

Comment 4

16 years ago
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 
(Reporter)

Comment 5

16 years ago
Created attachment 81138 [details]
screenshot of page for non-norwegian readers describing steps to reproduce
(Reporter)

Comment 6

16 years ago
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.
(Reporter)

Comment 7

16 years ago
just checked with build id 2002042510, same reproducible result.

Comment 8

16 years ago
Win2000, Build 2002042510

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

Comment 9

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

then it has nothing to do with this bug

Comment 10

16 years ago
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...
(Reporter)

Comment 11

16 years ago
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.

Updated

16 years ago
Keywords: hang

Updated

16 years ago
Summary: selecting text rappidly freezes mozilla → selecting text rapidly freezes mozilla

Updated

16 years ago
Target Milestone: --- → mozilla1.0

Comment 12

16 years ago
I seem to be unable to reproduce this on OS X Mozilla 1.0 branch build 2002042605 
(Reporter)

Comment 13

16 years ago
Created attachment 81274 [details]
tar file containing a snapshot of the page
(Reporter)

Comment 14

16 years ago
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

16 years ago
Created attachment 81280 [details]
stacktrace during hang

Comment 16

16 years ago
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
Assignee: mjudge → jkeiser
Status: UNCONFIRMED → NEW
Component: Selection → HTMLFrames
Ever confirmed: true
OS: Linux → All
QA Contact: tpreston → amar

Comment 17

16 years ago
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.
Summary: selecting text rapidly freezes mozilla → drag-selecting between table cells rapidly freezes mozilla
(Assignee)

Comment 18

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

Comment 19

16 years ago
Created attachment 81291 [details]
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

16 years ago
Created attachment 81301 [details]
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.
Attachment #81291 - Attachment is obsolete: true
Those problems both relate to parser/content sink (though I'm betting on content
sink).
Assignee: jkeiser → harishd
Component: HTMLFrames → Parser
QA Contact: amar → moied

Comment 22

16 years ago
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.
Assignee: harishd → attinasi
Component: Parser → Layout
QA Contact: moied → petersen

Comment 23

16 years ago
Changing QA Contact
QA Contact: petersen → moied

Updated

16 years ago
Priority: -- → P3

Comment 24

16 years ago
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

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

Bye,
Peter

Comment 26

16 years ago
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

16 years ago
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
See <http://bugzilla.mozilla.org/describekeywords.cgi>, keywords hang and crash.

Comment 29

16 years ago
Hi,
thanks.

Bye,
Peter

Comment 30

16 years ago
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

16 years ago
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
Assignee: attinasi → dbaron
Component: Layout → Style System
Keywords: regression
QA Contact: moied → ian
Summary: drag-selecting between table cells rapidly freezes mozilla → drag-selecting over unclosed font tags freezes mozilla

Comment 32

16 years ago
I meant between 18:35 / 18:40
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 ...
(Assignee)

Comment 34

16 years ago
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...
Status: NEW → ASSIGNED
Priority: P3 → P2
(Assignee)

Comment 35

16 years ago
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.
(Assignee)

Comment 36

16 years ago
Created attachment 82411 [details] [diff] [review]
patch (diff -u, for testing or checking in)
(Assignee)

Comment 37

16 years ago
Created attachment 82412 [details] [diff] [review]
patch (diff -uw, for review)
(Assignee)

Comment 38

16 years ago
> * 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|.
(Assignee)

Comment 39

16 years ago
... 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.)
(Assignee)

Comment 40

16 years ago
Created attachment 82413 [details] [diff] [review]
proposed branch fix (minimal changes to fix bug)
(Assignee)

Comment 41

16 years ago
Created attachment 82414 [details] [diff] [review]
proposed branch fix (minimal changes to fix bug)

with the comment changed too, this time...
Attachment #82413 - Attachment is obsolete: true

Comment 42

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

sr=waterson
Attachment #82411 - Flags: superreview+

Comment 43

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

sr=waterson
Attachment #82414 - Flags: superreview+

Comment 44

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

r=attinasi
Attachment #82414 - Flags: review+

Comment 45

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

r=attinasi
Attachment #82411 - Flags: review+

Comment 46

16 years ago
nominating for nsbeta1 (fix for rtm)
Keywords: nsbeta1
(Assignee)

Comment 47

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

Comment 48

16 years ago
cc'ing myself
(Assignee)

Comment 49

16 years ago
This patch caused bug 145224, which has been fixed.
(Reporter)

Comment 50

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

Comment 51

16 years ago
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?
(Reporter)

Comment 52

16 years ago
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.
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.
(Assignee)

Updated

16 years ago
Priority: P2 → P1
(Assignee)

Comment 54

16 years ago
Should be marked fixed since it's fixed on the trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Whiteboard: [open1.0.1]
(Assignee)

Comment 55

16 years ago
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.
Attachment #82414 - Attachment is obsolete: true

Updated

16 years ago
Attachment #82414 - Flags: approval+

Comment 56

16 years ago
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1+

Updated

16 years ago
Attachment #82411 - Flags: approval+
(Assignee)

Comment 57

16 years ago
Fix checked in to MOZILLA_1_0_BRANCH, 2002-06-20 20:04 PDT.
Keywords: mozilla1.0.1+ → fixed1.0.1
Whiteboard: [open1.0.1]
Target Milestone: mozilla1.0 → mozilla1.0.1

Comment 58

16 years ago
verified fixed on win20000 build -  2002-07-01-08-1.0 branch 
changing  keyword to verified1.0.1
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.1 → verified1.0.1
You need to log in before you can comment on or make changes to this bug.