Closed
Bug 404192
Opened 17 years ago
Closed 16 years ago
Crash [@ nsCaret::UpdateCaretRects] with caret browsing, xul titlebar focusing and blurring
Categories
(Core :: Layout, defect, P4)
Core
Layout
Tracking
()
VERIFIED
FIXED
People
(Reporter: martijn.martijn, Assigned: roc)
References
Details
(Keywords: crash, regression, testcase)
Crash Data
Attachments
(2 files, 1 obsolete file)
337 bytes,
application/xhtml+xml
|
Details | |
1.45 KB,
patch
|
MatsPalmgren_bugz
:
review+
MatsPalmgren_bugz
:
superreview+
|
Details | Diff | Splinter Review |
See testcase, which crashes after 200ms if you have caret browsing turned on. This regressed between 2007-08-12 and 2007-08-13: 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=2007-08-12+04&maxdate=2007-08-13+21&cvsroot=%2Fcvsroot My guess is a regression from bug 335560.
Reporter | ||
Comment 1•17 years ago
|
||
http://crash-stats.mozilla.com/report/index/bd8b8c66-9546-11dc-b9c5-001a4bd43e5c 0 nsCaret::UpdateCaretRects(nsIFrame*, int) mozilla/layout/base/nsCaret.cpp:1113 1 nsCaret::DrawAtPositionWithHint(nsIDOMNode*, int, nsFrameSelection::HINT, unsigned char, int) mozilla/layout/base/nsCaret.cpp:601 2 nsCaret::DrawCaret(int) mozilla/layout/base/nsCaret.cpp:1036 3 nsCaret::StartBlinking() mozilla/layout/base/nsCaret.cpp:537 4 nsCaret::NotifySelectionChanged(nsIDOMDocument*, nsISelection*, short) mozilla/layout/base/nsCaret.cpp:480 5 nsTypedSelection::NotifySelectionListeners() mozilla/layout/generic/nsSelection.cpp:7412 6 nsFrameSelection::NotifySelectionListeners(short) mozilla/layout/generic/nsSelection.cpp:2859 7 nsTypedSelection::AddRange(nsIDOMRange*) mozilla/layout/generic/nsSelection.cpp:5777 8 nsEventStateManager::MoveCaretToFocus() mozilla/content/events/src/nsEventStateManager.cpp:5313 9 nsGenericHTMLElement::SetElementFocus(int) mozilla/content/html/content/src/nsGenericHTMLElement.cpp:3086 10 nsGenericHTMLElement::Focus() mozilla/content/html/content/src/nsHTMLTextAreaElement.cpp:300 11 nsGenericHTMLElementTearoff::Focus() mozilla/content/html/content/src/nsGenericHTMLElement.cpp:196 12 NS_InvokeByIndex_P mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101 13 XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2339 14 GetPropertyTreeChild mozilla/js/src/jsscope.c:912
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.9?
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Priority: -- → P4
Comment 2•17 years ago
|
||
I can't get this to crash for me at all.
Reporter | ||
Comment 3•17 years ago
|
||
Still crashes for me in current trunk build (don't forget the caret browsing part).
Comment 4•17 years ago
|
||
I must admit that I don't really understand the underlying cause here, and the result on this page is not really ideal (the caret ends up sticking out of the upper left corner of the page) but at least with this patch we don't crash. We end up with an nsScrollPortView with no first child, which means that GetScrolledView returns null. roc, is this good enough, or do I need to dig deeper to figure out why there's no scrolled view?
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #294303 -
Flags: superreview?(roc)
Attachment #294303 -
Flags: review?(roc)
Comment 5•17 years ago
|
||
Oh, and I was totally forgetting the caret browsing part. Thanks for the heads up!
Updated•17 years ago
|
Whiteboard: [patch]
Assignee | ||
Comment 6•17 years ago
|
||
I would really like to know why there's no scrolled view. That probably means a corrupt frame hierarchy leading to other problems.
Comment 7•17 years ago
|
||
Giving this to roc to understand how we end up without a scrolled view. I see us create a nsXULScrollFrame as a result of dealing with the xul:titlebar but we never call SetScrolledView on the resulting view. Then (and I haven't confirmed this) we try to focus the HTML element, which is impossible, so we select its first child, which is the nsXULScrollFrame, leading to the crash (since there's no scrolled view). FWIW, this all seems reasonable to me, as I understand things, there's no reason to create a view for the titlebar until we actually need to scroll it.
Assignee: mrbkap → roc
Status: ASSIGNED → NEW
Assignee | ||
Comment 8•17 years ago
|
||
The titlebar frame creates its own view which becomes the first child of the scrollframe's view. The actual scrollable view becomes the second child of the scrollframe's view. Whee!
Assignee | ||
Comment 9•17 years ago
|
||
Simple fix. This got out of sync with nsHTMLContainerFrame::CreateViewForFrame. The code duplication is unfortunate but will go away when we get rid of views post-1.9.
Attachment #294303 -
Attachment is obsolete: true
Attachment #295714 -
Flags: superreview?(mats.palmgren)
Attachment #295714 -
Flags: review?(mats.palmgren)
Attachment #294303 -
Flags: superreview?(roc)
Attachment #294303 -
Flags: review?(roc)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [patch] → [needs review]
Comment 10•17 years ago
|
||
Comment on attachment 295714 [details] [diff] [review] fix Looks good, r+sr=mats Maybe add a comment to both nsHTMLContainerFrame::CreateViewForFrame and nsBoxFrame::CreateViewForFrame saying that "please keep this in sync with ..." ?
Attachment #295714 -
Flags: superreview?(mats.palmgren)
Attachment #295714 -
Flags: superreview+
Attachment #295714 -
Flags: review?(mats.palmgren)
Attachment #295714 -
Flags: review+
Updated•17 years ago
|
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [needs review]
Assignee | ||
Comment 12•16 years ago
|
||
checked in with that comment.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Flags: in-testsuite?
Whiteboard: [needs landing]
Reporter | ||
Comment 13•16 years ago
|
||
Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012204 Minefield/3.0b3pre
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
Crash Signature: [@ nsCaret::UpdateCaretRects]
Comment 14•10 years ago
|
||
Landed a crashtest: https://hg.mozilla.org/integration/mozilla-inbound/rev/30ea82feef41
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•