Closed
Bug 306070
Opened 19 years ago
Closed 19 years ago
middle-click paste and drag/drop after end of line in scrolled textarea go to wrong place offset by scroll position
Categories
(Core :: Layout: Form Controls, defect)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
People
(Reporter: dbaron, Assigned: sharparrow1)
References
Details
Attachments
(1 file)
4.58 KB,
patch
|
Details | Diff | Splinter Review |
Both middle-click paste and drag-drop to a blank line in a textarea that's scrolled away from the top of the page paste the text into the wrong place, seemingly offset by the scroll offset. Buggy in: Linux debug+opt build, CVS pull, ~8am 2005-08-25 Steps to reproduce: 1. Paste the following text in the "Additional Comments" field above: 0 5 10 15 2. scroll the textarea to the bottom 3. double click the word "blocks" above the text area 4. Either middle-click at line 8 (X11-only) or drag the highlighted word to line 8 (probably cross platform, but I haven't tested) Expected results: word ends up on line 8 Actual results: word ends up on line 3.
Comment 1•19 years ago
|
||
Probably a regression from bug 303399. Taking. I'll investigate it tomorrow.
Assignee: nobody → uriber
Comment 2•19 years ago
|
||
The drag case is reproducable on mac => All/All.
OS: Linux → All
Hardware: PC → All
Comment 3•19 years ago
|
||
This works in 2005-08-22-07 and is broken in 2005-08-23-07. Bonsai range: 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=2005-08-22+07%3A00%3A00&maxdate=2005-08-23+07%3A00%3A00&cvsroot=%2Fcvsroot I'll bet $5 this is the event->point change from bug 296036. Though it could also be bug 305239, the "offset by scroll position" part makes me think event coordinates.
Blocks: 296036
Reporter | ||
Comment 4•19 years ago
|
||
This actually affects any paste after the end of the line, not just those on blank lines (where all the space is after the end of the line). Also, you may run into bug 240416 while testing drag/drop into textareas. I did.
Summary: middle-click paste and drag/drop to blank line in scrolled textarea go to wrong place offset by scroll position → middle-click paste and drag/drop after end of line in scrolled textarea go to wrong place offset by scroll position
Comment 5•19 years ago
|
||
(In reply to comment #3) > I'll bet $5 this is the event->point change from bug 296036. > Though it could also be bug 305239, the "offset by scroll position" part makes > me think event coordinates. Hmmm... I don't see how this can be 305239 - the code affected there should never be entered in these cases. So my money goes with bz. I guess I'm so used to causing regressions I volunteered for this one too early... un-taking for now, until proven otherwise.
Assignee: uriber → nobody
Updated•19 years ago
|
Flags: blocking1.9a1?
Assignee | ||
Comment 6•19 years ago
|
||
Taking. I'm working on a fix, but I'm not sure how long it will take.
Status: NEW → ASSIGNED
Assignee | ||
Updated•19 years ago
|
Assignee: nobody → sharparrow1
Status: ASSIGNED → NEW
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•19 years ago
|
||
The part that actually fixes this bug is the nsLayoutUtils change. The other change fixes a bug I found but haven't filed.
Attachment #194000 -
Flags: review?(roc)
I'm not sure the nsLayoutUtils change is right. I had assumed the comment for GetEventCoordinatesForNearestView was what we wanted: * The "nearest view" is the view returned by nsFrame::GetOffsetFromView. If you change that, I think you'll mess up other callers. See e.g. http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#1504
Assignee | ||
Comment 9•19 years ago
|
||
I've been investigating it more carefully. GetContentAndOffsetsFromPoint is really expecting input in the GetClosestView coordinate system. The place you mention is the only other place that is calling that function after the patch for Bug 305944 gets checked in. As near as I can tell, the code you cited (http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#1484) is supposed to allow selection to work with clicks to the right of a moz-user-select=all box when it's at the end of a line. However, it doesn't work. Therefore, I'm in favor of replacing that whole chunk of code with "disableDragSelect = PR_TRUE;" like the XXX comment suggests. If you're OK with that, I'll make a new patch that does that.
(In reply to comment #9) > I've been investigating it more carefully. GetContentAndOffsetsFromPoint is > really expecting input in the GetClosestView coordinate system. Actually, looking at the code, I believe GetContentAndOffsetsFromPoint is confused about whether it takes GetClosestView-relative or GetOffsetFromView-relative coordinates... A prime candidate for fixing :-) Filed bug 306222. That's probably the root cause of this and other problems... > The place you > mention is the only other place that is calling that function after the patch > for Bug 305944 gets checked in. By "that function" I assume you mean GetEventCoordinatesForNearestView... I see here: http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#1752 http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#1760 as well as http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#1509. > As near as I can tell, the code you cited > (http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#1484) is > supposed to allow selection to work with clicks to the right of a > moz-user-select=all box when it's at the end of a line. However, it doesn't > work. Therefore, I'm in favor of replacing that whole chunk of code with > "disableDragSelect = PR_TRUE;" like the XXX comment suggests. If you're OK with > that, I'll make a new patch that does that. Wow, ContentContainsPoint is totally bogus code! I haven't done any testing (I'm at home without a trunk build) but if it's still possible to select content by clicking outside it (e.g. beyond the end of a line) then we still need code like this. Code that actually works, preferably :-). An easy fix here that wouldn't make anything worse would be to just change the call to GetOffsetFromView to GetClosestView at the same time as you change the behaviour of GetEventCoordinatesFromNearestView. But actually I fear the confusion in GetContentAndOffsetsFromPoint about the interpretation of its aPoint parameter makes any change here very risky. Maybe the safest thing to do is actually to go ahead and at least clean up the coordinate handling in GetContentAndOffsetsFromPoint.
Depends on: 306222
Actually http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#1511 can just be deleted since we're already setting 'view'. More about the confusion in GetContentAndOffsetsFromPoint: The code block at http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#2023 is clearly understanding aPoint to be relative to GetClosestView(). Unfortunately, for example, the code starting http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#2046 clearly puts thisRect in coordinates relative to GetOffsetFromView()'s view and then assumes aPoint is in the same coordinate space... nsTextFrame::GetContentAndOffsetsFromPoint/GetPosition(Slowly) has similar confusion. I think the way to proceed in direction of strictly increasing sanity is to make GetContentAndOffsetsFromPoint (and nsTextFrame::GetPosition(Slowly)) take frame-relative coordinates. That will eliminate most uses of GetEventCoordinatesFromNearestView and probably fix this bug. If we do that we can leave GetEventCoordinatesFromNearestView and its other callers unchanged for now. If that causes other regressions, at least we're moving in the direction of having better-specified methods so we know we're making progress in the right direction.
Comment 12•19 years ago
|
||
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20050904 Firefox/1.6a1 ID:2005090415 The check-in for bug 306222 seems to fix this for me.
Comment 13•19 years ago
|
||
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20050913 Firefox/1.6a1 ID:2005091311 This was fixed by bug 306222.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Flags: blocking1.9a1?
Resolution: --- → FIXED
Updated•19 years ago
|
Attachment #194000 -
Flags: review?(roc)
You need to log in
before you can comment on or make changes to this bug.
Description
•