Closed Bug 15360 Opened 25 years ago Closed 25 years ago

Drag Selecting, Clicking, then Clicking leaves caret cruft.

Categories

(Core :: DOM: Selection, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: kinmoz, Assigned: kinmoz)

Details

To reproduce this bug: 1. Drag select part of the URL in the Location (URL) field. 2. Now click in some text that is outside of the selection so that the caret appears. 3. Now click in another portion of the text so that the caret moves. Notice that the caret placed in #2 is still showing?
Status: NEW → ASSIGNED
Target Milestone: M11
Simon and Akkana, can you reproduce this on Mac and Linux?
The bug only seems to happen if you click in a text frame that lies within the region that is damaged when re-rendering frames that were selected. In the simple case (textfield, URL bar, etc.) there is only one text frame. If you highlight part of the text frame, then click outside the selection, within the same frame, the entire frame is invalidated and redrawn. The problem occurs because the caret is not using the same rendering context that the presShell uses to paint the frames. Instead, it creates it's own which draws directly to the screen. The presShell's context draws to an offscreen buffer which is then blitted to the screen, from the ViewManager's code which get's executed after the presShell paints it's frames and after the caret has already been drawn on the screen. In short, the blit erases the caret off the screen without the caret's knowledge. So when it comes time to erase the caret, it inverts the caret, causing it to appear on the screen. Simon, should we modify nsICaret so that we can pass in a rendering context to use? I don't think we can modify the view code to hide and show the caret since it knows nothing about nsIPresShell.
At one point the code did use the nsIRenderingContext that was passed in on a Refresh from the pres shell, but this created problems because the coordinate systems seemed to be different; it never worked correctly.
The rendering context passed to the presshell during a refresh ususally has a coordinate system relative to the view that is also passed. Since views can have subviews, the frame the caret draws itself in might be in a subview, in which case, the coordinate system has to be adjusted. I fixed a problem like this in the frame selection code ... perhaps it's the same problem. Where can I find all the code that used to use the passed in rendering context?
It's commented out in RefreshDrawCaret(). There used to be a Refresh() method in nsICaret, but I removed it. Backing up a few revs in that file will bring it back.
Whiteboard: I have a fix for this, I am waiting for code review feedback.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Fix checked in. Reviewed by troy@netscape.com, kipp@netscape.com, evaughan@netscape.com, and beard@netscape.com To fix the problem, I modified the presShell to shut off caret drawing before scrolling or repainting. layout/html/base/src/nsGfxScrollFrame.cpp - Renamed all occurrences of ScrollPositionChanged() to to ScrollPositionDidChange() to match changes made to nsIScrollPositionListener. - Added ScrollPositionWillChange() stub method. layout/html/base/src/nsPresShell.cpp - Added PresShellViewEventListener class that implements the nsIScrollPositionListener and nsICompositeListener view interfaces. This allows us to turn caret rendering on and off when scrolling and repainting to prevent leaving caret cruft on the screen. - PresShellViewEventListener is created in InitialReflow(), and released in ~PresShell(). view/public/Makefile.in view/public/makefile.win view/public/MANIFEST - Added nsICompositeListener to list of files to be exported. view/public/nsICompositeListener.h - New file. Implementers of this interface can register themselves with the view manager to receive notification before and after a view is composited/refreshed. view/public/nsIScrollPositionListener.h - Added a ScrollPositionWillChange() method. - Changed ScrollPositionChanged() to ScrollPositionDidChange(). view/public/nsIViewManager.h - Added AddCompositeListener() and RemoveCompositeListener methods. view/src/nsScrollingView.cpp view/src/nsScrollingView.h - Added NotifyScrollPositionWillChange() and NotifyScrollPositionDidChange() methods. - Modified HandleScrollEvent() to call NotifyScrollPositionWill/DidChange() methods. Added temporary offsetX and offsetY variables to allow us to calculate new offsets without changing mOffsetX and mOffsetY before notifications are sent out. view/src/nsScrollPortView.cpp - Changed ScrollPositionChanged() to ScrollPositionDidChange(). - Modified ScrollTo() to call the listener's ScrollPositionWillChange() method. view/src/nsViewManager.cpp view/src/nsViewManager.h - Added AddCompositeListener() and RemoveCompositeListener() methods. - Added support for CompositeListener notifications in the Refresh() methods for regions and rects. - Fix for bug #15901: [DOGFOOD] Selected text does not clear when you type Modified UpdateView() so that rects with zero width and height are no longer added to our damage region. r=beard@netscape.com
Doh. Could I please confirm as a formality that steps #2 and #3 involve clicking in a text field, such as within a form? (I don't see how to place an editing caret within the browser other than within a text field.)
The only way to reproduce this problem in the browser, is to do steps 1-3 in the URL/Location textfield at the top of the window, or by loading a form with a textarea or textfield. This bug also happened in the editor test bed (composer) and the mail message compose window.
Status: RESOLVED → VERIFIED
Whiteboard: I have a fix for this, I am waiting for code review feedback.
Thanks! Verified fixed on 1999102008 (Mac OS, 8.6), 1999101908 (Win32, NT 4.0SP5), and 1999101908 (Linux, RH 6.0/GNOME). [Checked within a URL text field, and within the editor.]
You need to log in before you can comment on or make changes to this bug.