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 (selection.cpp:863) 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: 4508) 9 com.apple.WebCore 0x0190284c KParts::Part::event(QEvent*) + 64 (KWQKPartsPart.h:56) 10 com.apple.WebCore 0x01911a40 QApplication::sendEvent(QObject*, QEvent*) + 64 (KWQApplication.h:38) 11 com.apple.WebCore 0x0166cd80 KHTMLView::viewportMousePressEvent(QMouseEvent*) + 736 (KWQPtrList.h:807) 12 com.apple.WebCore 0x015f0b74 KWQKHTMLPart::mouseDown(NSEvent*) + 532 (KWQKHTMLPart.mm:2611) 13 com.apple.WebCore 0x0164e7f4 -[WebCoreBridge mouseDown:] + 52 (WebCoreBridge.mm: 864) 14 com.apple.WebKit 0x0037d694 -[WebHTMLView mouseDown:] + 356 (WebHTMLView.m: 2683) 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 -------------------------------------------------------------------
Can't reproduce it, will leave it open for anyone else to try, please provide further insight in how to recreate it.
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 lists.apple.com/archives. 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.
Confirming it, cause a crash happened, but i can't reproduce.
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.
Same as <rdar://problem/4140688> assertion failure double-clicking text to select. The fix for that has been submitted. Moving this bug to FIXED.
The fix seems to be not in cvs, so cannot verify yet.
The fix is in WebCore/khtml/rendering/render_text.cpp, revision 183, which was committed on 27 June 2005.
It looks like a different issue then - I can still reproduce it with ToT (clean rebuild, render_text.cpp at revision 1.192). Reopening.
Created attachment 3674 [details] Newer crash log Line numbers have changed a lot, so here is a current crash log.
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.
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).
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 visible_position.h: // NOTE: UPSTREAM affinity will be used only if pos is at end of a wrapped line, // otherwise it will be converted to DOWNSTREAM
Created attachment 4192 [details] Automated test
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.
(there I go again saying "line" when I mean "paragraph"...)
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.
Committed.
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.
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 on attachment 4597 [details] RenderText::inlineBox fix Removing review+
Created attachment 4625 [details] atLineWrap Fixes caretRect in similar manner as inlineBox was. In fact, the logic is now shared in new function RenderText::atLineWrap.
Comment on attachment 4625 [details] atLineWrap Justin and Hyatt approved this. I added the automated test as editing/selection/doubleclick-crash.html.
Committed,
Verified, the assertion no longer fires!