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)
Tracking
()
VERIFIED
FIXED
M11
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?
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.
Comment 3•25 years ago
|
||
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?
Comment 5•25 years ago
|
||
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
Comment 7•25 years ago
|
||
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.
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Whiteboard: I have a fix for this, I am waiting for code review feedback.
Comment 9•25 years ago
|
||
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.
Description
•