Bug 3739 - Unreproducible - Assertion failure in isEqualIgnoringAffinity on double-click
Summary: Unreproducible - Assertion failure in isEqualIgnoringAffinity on double-click
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 412
Hardware: Macintosh OS X 10.4
: P2 Major
Assignee: David Harrison
URL: http://bugzilla.opendarwin.org/show_b...
Depends on:
Reported: 2005-06-27 12:44 PDT by Alexey Proskuryakov
Modified: 2005-11-08 13:17 PST (History)
0 users

See Also:

Newer crash log (21.09 KB, text/plain)
2005-08-30 09:35 PDT, Alexey Proskuryakov
no flags Details
Force DOWNSTREAM at end of paragraph (1.21 KB, patch)
2005-10-04 08:34 PDT, Alexey Proskuryakov
harrison: review-
Details | Formatted Diff | Diff
Automated test (2.85 KB, patch)
2005-10-04 09:11 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
RenderText::inlineBox fix (1.46 KB, patch)
2005-11-04 10:57 PST, David Harrison
no flags Details | Formatted Diff | Diff
atLineWrap (4.75 KB, patch)
2005-11-07 16:50 PST, David Harrison
harrison: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2005-06-27 12:44:20 PDT
In development build of ToT WebKit, an assertion often fires after a double-click in an empty area.

Steps to reproduce:
1. Go to http://bugzilla.opendarwin.org/show_bug.cgi?id=3510 (somehow, I can most reliably 
reproduce this issue on that page ;-) ) 
2. Scroll down to the beginning of the description
3. Double-click after the end of the first paragraph (after "Accept-Language.")

Results: with a high probability, an assertion fires (causing a crash in development build).

Discussion: I have dumped the values causing the assertion to fire:

 a.m_deepPosition: "offset 129 of #text; value=If the system primary language (as set in International 
control panel) is Russian, then only "ru-ru" is 
sent in Accept-Language.

Steps to reproduce:
1. Set the system primary language to Russian (on my machine, the exact order is Russian, English, 
Japanese, Chinese Traditional, Chinese Simplified).
2. In Safari, go to "
 b.m_deepPosition: "offset 129 of #text; value=If the system primary language (as set in International 
control panel) is Russian, then only "ru-ru" is 
sent in Accept-Language.

Steps to reproduce:
1. Set the system primary language to Russian (on my machine, the exact order is Russian, English, 
Japanese, Chinese Traditional, Chinese Simplified).
2. In Safari, go to "
 a.m_affinity: 0
 b.m_affinity: 1

Here is the crash log:
0   com.apple.WebCore        	0x01883200 khtml::isEqualIgnoringAffinity(khtml::VisiblePosition 
const&, khtml::VisiblePosition const&) + 352 (visible_position.cpp:396)
1   com.apple.WebCore        	0x0188b084 khtml::isEndOfParagraph(khtml::VisiblePosition const&) + 
92 (visible_units.cpp:663)
2   com.apple.WebCore        	0x0187e30c khtml::Selection::validate(khtml::ETextGranularity) + 1644 
3   com.apple.WebCore        	0x0187f290 khtml::Selection::expandUsingGranularity
(khtml::ETextGranularity) + 72 (selection.cpp:499)
4   com.apple.WebCore        	0x0165c4d8 KHTMLPart::selectClosestWordFromMouseEvent
(QMouseEvent*, DOM::NodeImpl*, int, int) + 260 (khtml_part.cpp:4582)
5   com.apple.WebCore        	0x0165c630 KHTMLPart::handleMousePressEventDoubleClick
(khtml::MousePressEvent*) + 216 (khtml_part.cpp:4608)
6   com.apple.WebCore        	0x0165fc7c KHTMLPart::khtmlMousePressEvent
(khtml::MousePressEvent*) + 616 (khtml_part.cpp:4721)
7   com.apple.WebCore        	0x015f1fe4 KWQKHTMLPart::khtmlMousePressEvent
(khtml::MousePressEvent*) + 460 (KWQKHTMLPart.mm:2012)
8   com.apple.WebCore        	0x0165b60c KHTMLPart::customEvent(QEvent*) + 88 (khtml_part.cpp:
9   com.apple.WebCore        	0x0190284c KParts::Part::event(QEvent*) + 64 (KWQKPartsPart.h:56)
10  com.apple.WebCore        	0x01911a40 QApplication::sendEvent(QObject*, QEvent*) + 64 
11  com.apple.WebCore        	0x0166cd80 KHTMLView::viewportMousePressEvent(QMouseEvent*) + 
736 (KWQPtrList.h:807)
12  com.apple.WebCore        	0x015f0b74 KWQKHTMLPart::mouseDown(NSEvent*) + 532 
13  com.apple.WebCore        	0x0164e7f4 -[WebCoreBridge mouseDown:] + 52 (WebCoreBridge.mm:
14  com.apple.WebKit         	0x0037d694 -[WebHTMLView mouseDown:] + 356 (WebHTMLView.m:
15  com.apple.AppKit         	0x93686d28 -[NSWindow sendEvent:] + 4616
16  com.apple.Safari         	0x0001d6bc 0x1000 + 116412
17  com.apple.AppKit         	0x9362ff5c -[NSApplication sendEvent:] + 4172
18  com.apple.Safari         	0x0001a6a4 0x1000 + 104100
19  com.apple.AppKit         	0x936273f0 -[NSApplication run] + 508
20  com.apple.AppKit         	0x93717c1c NSApplicationMain + 452
21  com.apple.Safari         	0x00002700 0x1000 + 5888
22  com.apple.Safari         	0x00057190 0x1000 + 352656
Comment 1 Joost de Valk (AlthA) 2005-06-27 23:32:31 PDT
Can't reproduce it, will leave it open for anyone else to try, please provide further insight in how to 
recreate it.
Comment 2 Alexey Proskuryakov 2005-06-28 10:40:01 PDT
For me, crashing usually occurs when double-clicking in the whitespace after a paragraph end if that 
paragraph uses a non-proportional font. Besides Bugzilla, I've now reproduced it with messages in 

I have tried changing the font (Monaco or Courier at different sizes), but the results were the same.

I have mentioned it, but didn't stress - AFAIK, one needs to run a development build of WebKit (set-
webkit-configuration --development) to see assertions.

Even if this issue stays unreproducible, perhaps the crash log combined with the data could lead someone 
to discovering its cause? There are some hints in isEqualIgnoringAffinity() code.
Comment 3 Joost de Valk (AlthA) 2005-06-30 11:39:56 PDT
Confirming it, cause a crash happened, but i can't reproduce.
Comment 4 John Sullivan 2005-06-30 11:44:40 PDT
There's another report of this in radar bug <rdar://problem/4054701>. That case involves using 
VoiceOver and Mail, with these steps:

1. Using TOT build of Safari, launch Mail from terminal. Type three words in the compose window.
2. command F5 to launch VoiceOver
3. Bring VoiceOver cursor into the compose portion of the window.  Ctrl-Option-end, then ctrl-option-
shift-down arrow.
4. Read word by word using ctrl-option-right arrow until you reach the last word.
5. Upon reaching the last word, WebCore crashes.

Dave Harrison confirms in 4054701 that there are no ill effects from this assert on deployment builds, 
but it does indicate an unexpected condition, and should be addressed. Assigning this to him.
Comment 5 David Harrison 2005-06-30 15:04:27 PDT
Same as <rdar://problem/4140688> assertion failure double-clicking text to select.

The fix for that has been submitted.  Moving this bug to FIXED.
Comment 6 Alexey Proskuryakov 2005-07-02 03:42:02 PDT
The fix seems to be not in cvs, so cannot verify yet.
Comment 7 David Harrison 2005-08-29 16:21:54 PDT
The fix is in WebCore/khtml/rendering/render_text.cpp, revision 183, which was committed on 27 June 
Comment 8 Alexey Proskuryakov 2005-08-30 09:34:00 PDT
It looks like a different issue then - I can still reproduce it with ToT (clean rebuild, render_text.cpp at 
revision 1.192). Reopening.
Comment 9 Alexey Proskuryakov 2005-08-30 09:35:24 PDT
Created attachment 3674 [details]
Newer crash log

Line numbers have changed a lot, so here is a current crash log.
Comment 10 David Harrison 2005-09-01 11:05:59 PDT
This should be fixed by the changes for

<rdar://problem/4054701> "assertion failure in khtml::isEqualIgnoringAffinity using VoiceOver in new 
Mail message"

I've just committed that fix.
Comment 11 Alexey Proskuryakov 2005-09-01 11:50:41 PDT
Unfortunately, it still crashes for me (render_text.cpp at version 1.195, clean rebuild). The stack trace 
hasn't changed (except for line numbers, of course).
Comment 12 Alexey Proskuryakov 2005-10-04 08:34:32 PDT
Created attachment 4191 [details]
Force DOWNSTREAM at end of paragraph

This is almost a shot in the dark, but:
1) it fixes the (100% reproducible) assertion failure for me;
2) and it really implements what is suggested by a comment in
    // NOTE: UPSTREAM affinity will be used only if pos is at end of a wrapped
    // otherwise it will be converted to DOWNSTREAM
Comment 13 Alexey Proskuryakov 2005-10-04 09:11:09 PDT
Created attachment 4192 [details]
Automated test
Comment 14 David Harrison 2005-10-04 12:39:50 PDT
Comment on attachment 4191 [details]
Force DOWNSTREAM at end of paragraph

This patch does not address the root cause of the problem, which is that we
need to define how affinity applies to newline characters in text nodes with
whitespace:PRE style.	The assert fires because of an inconsistency:
RenderText::inlineBox() acts as if affinity applies to newlines in preformatted
text, but endOfParagraph() does not (it always produces DOWNSTREAM affinity).

The better fix is to decide a policy and make it consistent.  My gut says to
have affinity NOT apply in this case, because affinity is a "line wrap" concept
not a paragraph break concept (even if that break is within a text node, it is
not the same as "line wrap").  This would mean modifying
RenderText::inlineBox() to heed newlines in whitespace:PRE.  It probably means
other changes, as well, because this nuance was AFAIK not considererd when
writing the affinity-related code.  For example,  I locally fixed
RenderText::inlineBox(), and single-clicking at the end of the line leaves the
cursor blinking at the beginning of the second line.
Comment 15 David Harrison 2005-10-04 12:42:14 PDT
(there I go again saying "line" when I mean "paragraph"...)
Comment 16 David Harrison 2005-11-04 10:57:10 PST
Created attachment 4597 [details]
RenderText::inlineBox fix

This patch addresses the root cause of the problem by making
RenderText::inlineBox() cope with the fact that in white-space:pre text the
newline characters are not part of any InlineTextBox... they lie "between"
them.	Now DOWNSTREAM affinity selects the next text box only if the offset
that is past the current box is actually _in_ the next box.
Comment 17 David Harrison 2005-11-04 13:02:03 PST
Comment 18 David Harrison 2005-11-04 13:07:49 PST
Now I have to look at the follow-on problem: single-clicking at the end of the line leaves the
cursor blinking at the beginning of the next line.
Comment 19 Geoffrey Garen 2005-11-04 22:15:26 PST
Dave, do you want to close this bug, and open a new bug for the follow-on you mentioned? (This bug 
misleadingly shows up in the "to be landed" queue.)
Comment 20 David Harrison 2005-11-07 06:19:40 PST
Comment on attachment 4597 [details]
RenderText::inlineBox fix

Removing review+
Comment 21 David Harrison 2005-11-07 16:50:25 PST
Created attachment 4625 [details]

Fixes caretRect in similar manner as inlineBox was.  In fact, the logic is now
shared in new function RenderText::atLineWrap.
Comment 22 David Harrison 2005-11-07 16:52:03 PST
Comment on attachment 4625 [details]

Justin and Hyatt approved this.   I added the automated test as
Comment 23 David Harrison 2005-11-07 16:52:56 PST
Comment 24 Alexey Proskuryakov 2005-11-08 13:17:56 PST
Verified, the assertion no longer fires!