607857 - caret rect gets zeroed if frame containing caret is dirty

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Timothy Nikkel (:tnikkel)]

nsCaret::GetGeometryForFrame ignores the nsresult of nsIFrame::GetPointFromOffset, but it returns an error and doesn't update the outparams if the frame is dirty. This position the caret rect as the start of the frame. I've never seen the caret actually get drawn there because we usually get another call to position the caret, but I've heard other people say that they've seen the caret jump to the start of a textbox (on Mac).

We invalidate the caret rect in both PresShell::WillDoReflow and PresShell::DidDoReflow, so I don't think this can cause any harm, but it will certainly save some painting, and maybe fix a visible bug.

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Comment 2

[Jesse Ruderman]

I see the caret get drawn there while typing into the prompt() textbox on Mac OS X 10.5, for example.

Comment 3

[Michael Wu [:mwu]]

This patch fixes my problem on OSX. Check it in!

Comment 4

[(no longer active)]

(In reply to comment #2)
> I see the caret get drawn there while typing into the prompt() textbox on Mac
> OS X 10.5, for example.

Yeah, this reminds me that I was planning to file a bug on that, but I'm happy to see that you have a fix before I even file the bug!  :-)

Comment 5

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/mozilla-central/rev/c198cabd2743

Comment 6

[Timothy Nikkel (:tnikkel)]

I've never reproduced a problem here, if someone who can can think of a way to test this, then great!

Comment 7

[(no longer active)]

To test this, we need to be able to capture the keypress event right after it's been handled by the editor.  This is, for example, a stack which shows GetPointFromOffset failing:

#0  nsCaret::GetGeometryForFrame (this=0x11f14d940, aFrame=0x1014c91a0, aFrameOffset=6, aRect=0x11f14d998, aBidiIndicatorSize=0x7fff5fbf593c) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsCaret.cpp:368
#1  0x0000000113ea8cf8 in nsCaret::UpdateCaretRects (this=0x11f14d940, aFrame=0x1014c91a0, aFrameOffset=6) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsCaret.cpp:1084
#2  0x0000000113eaa4a6 in nsCaret::DrawAtPositionWithHint (this=0x11f14d940, aNode=0x126d1b310, aOffset=6, aFrameHint=nsFrameSelection::HINTLEFT, aBidiLevel=128 '\200', aInvalidate=1) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsCaret.cpp:720
#3  0x0000000113eaa98f in nsCaret::DrawCaret (this=0x11f14d940, aInvalidate=1) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsCaret.cpp:1074
#4  0x0000000113eaaab6 in nsCaret::StartBlinking (this=0x11f14d940) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsCaret.cpp:660
#5  0x0000000113eab3a6 in nsCaret::SetCaretVisible (this=0x11f14d940, inMakeVisible=1) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsCaret.cpp:336
#6  0x0000000113ff7462 in StCaretHider::~StCaretHider (this=0x7fff5fbf5bb0) at nsCaret.h:323
#7  0x000000011469e5fc in nsEditor::EndPlaceHolderTransaction (this=0x11b2cea30) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/base/nsEditor.cpp:933
#8  0x0000000114685ff3 in nsAutoPlaceHolderBatch::~nsAutoPlaceHolderBatch (this=0x7fff5fbf5ca0) at nsEditorUtils.h:65
#9  0x000000011467e564 in nsPlaintextEditor::TypedText (this=0x11b2cea30, aString=@0x7fff5fbf5ce0, aAction=0) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsPlaintextEditor.cpp:447
#10 0x00000001146838c3 in nsPlaintextEditor::HandleKeyPressEvent (this=0x11b2cea30, aKeyEvent=0x11b5adef8) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/text/nsPlaintextEditor.cpp:416
#11 0x00000001146aba50 in nsEditorEventListener::KeyPress (this=0x11b542d50, aKeyEvent=0x11b5ade60) at /Users/ehsanakhgari/moz/mozilla-central/editor/libeditor/base/nsEditorEventListener.cpp:344
#12 0x000000011430640d in DispatchToInterface (aEvent=0x11b5ade60, aListener=0x11b542d50, aMethod={__pfn = 0x31, __delta = 0}, aIID=@0x114c7b6e0) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsEventListenerManager.cpp:175
#13 0x0000000114307c11 in nsEventListenerManager::HandleEventInternal (this=0x11b540c30, aPresContext=0x1019b6c00, aEvent=0x7fff5fbf69b0, aDOMEvent=0x7fff5fbf61e0, aCurrentTarget=0x11f129920, aFlags=518, aEventStatus=0x7fff5fbf61e8, aPusher=0x7fff5fbf6200) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsEventListenerManager.cpp:1206
#14 0x0000000114337e67 in nsEventListenerManager::HandleEvent (this=0x11b540c30, aPresContext=0x1019b6c00, aEvent=0x7fff5fbf69b0, aDOMEvent=0x7fff5fbf61e0, aCurrentTarget=0x11f129920, aFlags=518, aEventStatus=0x7fff5fbf61e8, aPusher=0x7fff5fbf6200) at nsEventListenerManager.h:146
#15 0x0000000114338012 in nsEventTargetChainItem::HandleEvent (this=0x1013be310, aVisitor=@0x7fff5fbf61d0, aFlags=518, aMayHaveNewListenerManagers=0, aPusher=0x7fff5fbf6200) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsEventDispatcher.cpp:212
#16 0x0000000114336423 in nsEventTargetChainItem::HandleEventTargetChain (this=0x1013be770, aVisitor=@0x7fff5fbf61d0, aFlags=518, aCallback=0x7fff5fbf6310, aMayHaveNewListenerManagers=0, aPusher=0x7fff5fbf6200) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsEventDispatcher.cpp:341
#17 0x000000011433661d in nsEventTargetChainItem::HandleEventTargetChain (this=0x1013be770, aVisitor=@0x7fff5fbf61d0, aFlags=6, aCallback=0x7fff5fbf6310, aMayHaveNewListenerManagers=0, aPusher=0x7fff5fbf6200) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsEventDispatcher.cpp:395
#18 0x00000001143370ba in nsEventDispatcher::Dispatch (aTarget=0x11f129920, aPresContext=0x1019b6c00, aEvent=0x7fff5fbf69b0, aDOMEvent=0x0, aEventStatus=0x7fff5fbf65ec, aCallback=0x7fff5fbf6310, aTargets=0x0) at /Users/ehsanakhgari/moz/mozilla-central/content/events/src/nsEventDispatcher.cpp:628
#19 0x0000000113efe401 in PresShell::HandleEventInternal (this=0x11f14c5e0, aEvent=0x7fff5fbf69b0, aView=0x11f14b9d0, aStatus=0x7fff5fbf65ec) at /Users/ehsanakhgari/moz/mozilla-central/layout/base/nsPresShell.cpp:6962

I really don't think this is possible to capture in an automated test...

Maybe we should resort to a Litmus test?