289792 - funky selection behaviour

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Caleb]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050408 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050408 Firefox/1.0+

There seem to be some serious issues with selections (I think that this also
happens in the v1.0.2 offical builds).

Case 1:

I've noticed a few strange things, one of them on allmusic.com (I've attached a
copy of a testcase for this)

Open the testcase, and start your selection anywhere, but make sure to keep
dragging the mouse until you reach the end of "Read More...", and drag the mouse
 after that last dot.

You will notice that the selection will "bounce back". This does not happen in IE.
In IE you can keep dragging your mouse around the bottom of the screen (under
that bit of text) and the selection will remain whole, as it should.

Case 2:

The other issue, which I think that could be related (if not a new bug can be
opened), is what happens when you select a bit of text in the talkback section
of an Israeli news site.

Go here: http://www.ynetnews.com/articles/0,7340,L-3070407,00.html
Wait for the talkbacks to load, open any talkback (by pressing on its topic name).
Now put your mouse after the Close image (it's an X) and start selecting that
whole line, then GENTLY pull your mouse up to select the rest of the talkback
text and WHOOPY, notice how the page bumps (scrolls) to the top!

Once again, in IE, I can move the selection (and manipulate) as I want to
without the page bouncing back to the top.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Caleb]

Attachment: testcase 1

testcase for case1 (just a copy of some HTML from www.allmusic.com, in case it
changes)

Comment 2

[Gabriel Chadwick]

I confirm the issues in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.8b2) Gecko/20050409 Firefox/1.0+

But i think these to issue are two separate bugs.

Comment 3

[simon annear]

confirmed tescase 1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.8b2) Gecko/20050316

Comment 4

[Hermann Schwab]

Attachment: reduced testcase 1

just took the well prepared testcase 1 of the reporter, and reduced it further,
and added borders and duplicated the div.
testcase depends on position:absolute

Steps to test:

Select 'more...' in the last line, move mouse from 'm' to the last dot, and
then beyound, and the previously selected area isnt selected anymore, the
selection reverts, doen´t go from the 'm' to the right, but instead to the
left, following some lines up, finishing with the x position it should have.
the y postion depends on where you do it, in the first or second div, having
only absolute üpositions as differebce.

Comment 5

[Hermann Schwab]

http://www.ynetnews.com/articles/0,7340,L-3070407,00.html

original steps were working sometimes somehow, found another way:

Wait for the talkbacks to load, open any talkback (by pressing on its topic name).
Now put your mouse ABOVE the Close image (it's an X) and LEFT CLICK, 
then GENTLY pull your mouse LEFT AND UP,
and WHOOPY, notice how the page bumps (scrolls) to the top!

This works in the region following the date, try clicking just right of
(04.10.05), move a pixel left and up, maybe you don´t have to move at all.

Code:

<div style="overflow: hidden; width: 220px;" class="text12g">
<table cellpadding="0" cellspacing="0"><tbody><tr>
<td>D Zucker , &nbsp;</td>
<td></td><td>Ramat Gan, Israel&nbsp;</td>
<td></td>
<td>(04.10.05)</td>
<td></td>
</tr></tbody></table></div>

<table border="0" cellpadding="0" cellspacing="0" height="9" width="375">
<tbody><tr><td>&nbsp;</td></tr></tbody></table>

</span>

Doesn´t depend on JS, I had JS enabled to open the talkback, then disabled JS
before testing to see if the jumping is JS related, jumping also happened with
JS disabled.
I also didn´t have flash installed, the top of the page was showing the broken
plugin symbol, I assume flash plugin needed.

Comment 6

[Hermann Schwab]

found regression
working Build ID 20050402, 
failing to load Build ID 20050403
showing bug Build ID 2005040406

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050404

steps to repeat:
1. load URL  http://www.ynetnews.com/articles/0,7340,L-3070407,00.html
2. enable JS, if not enabled
3. at the bottom of the article are talkbacks, click on the second or another
one, let it open
4. JS can be disabled, doesn´t matter.
5. see example below, 2nd talkback: just click right of the date (04.10.05)
move the pointer 2 pixels in some direction, and you are at top.


Example 2nd talkback:

 	2.  Move from the Gush  	
  		 	
Due to bad traffic problems, many people will have to move to TA, RG and
Givataim so they don't loose their jobs by being late to work.
D Zucker ,  		Ramat Gan, Israel  		(04.10.05)	
 
Recommend this talkback click here 	Add talkback  		Close  X	

Comment 7

[Hermann Schwab]

check-ins in timeframe:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-04-02+00%3A00&maxdate=2005-04-04+07%3A00&cvsroot=%2Fcvsroot

just a guess: Bug 288117 Eliminate nsScrollBoxFrame
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-04-03+14%3A00&maxdate=2005-04-03+14%3A05&cvsroot=%2Fcvsroot

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix part 1

This patch fixes the selection problem in absolute frames. However, it doesn't
fix the URL testcase. We need a reduced testcase for that ... any volunteers to
make one?

Comment 9

[Caleb]

Attachment: testcase 2

Here's my attempt at creating a cleaner testcase 2.

If you can reduce it further, please do :)

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: more reduced testcase

Comment 11

[Steve England [:stevee]]

Attachment: a little more reduced

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This fixes both testcases. It's simply a matter of choosing the right view to
translate coordinates to; the event coordinates are always relative to the view
returned by GetOffsetFromView (which is really stupid, but that's the way it is
for now).

The viewmanager changes are just a sanity thing to protect us from possible
bugs in frame code that don't restore event coordinates along some path. (I
didn't find any, though.)

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 180670 [details] [diff] [review]
fix

>Index: layout/generic/nsBlockFrame.cpp
>-        aEvent->point -= resultFrameView->GetOffsetTo(closestView);
>+        aEvent->point += resultFrameParentView->GetOffsetTo(parentWithView);

That should still be a -=.  With that fixed, r+sr=bzbarsky.

>       }
> 
>       if (NS_POSITION_BEFORE_TABLE == result)
>       {
>         nsCOMPtr<nsISelectionController> selCon;
>         result = GetSelectionController(aPresContext, getter_AddRefs(selCon));
>         //get the selection controller
>         if (NS_SUCCEEDED(result) && selCon) 
>         {
>           PRInt16 displayresult;
>           selCon->GetDisplaySelection(&displayresult);
>           if (displayresult == nsISelectionController::SELECTION_OFF)

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 180670 [details] [diff] [review]
fix

small, well-understood fix for some event handling/selection regressions (and
possibly previously existing event handling/selection issues)

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 180670 [details] [diff] [review]
fix

a=asa

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 17

[Caleb]

reduced testcase 1 still has the same issue after testing the newest beast build.

testcase 2 is fixed, although I'm not sure whether selecting the date should
make the selection "stuck" on it. (you cannot drag the selection to top once you
select the date in the "a little more reduced" testcase).

Just select the date from right to left and keep dragging the mouse to top and
you will see that the selection won't move.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #17)
> reduced testcase 1 still has the same issue after testing the newest beast
> build.

Works for me. Beast only just finished cycling, are you sure you've got a build
with the complete fix?

> testcase 2 is fixed, although I'm not sure whether selecting the date should
> make the selection "stuck" on it. (you cannot drag the selection to top once
> you select the date in the "a little more reduced" testcase).

I think that's a previously known issue.

Comment 19

[Caleb]

Oh, my bad, I re-downloaded the newest BEAST build and testcase1 is fixed.

Comment 20

[Hermann Schwab]

thanks for the quick fix,
wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050415
from CREATURE Tinderbox

Comment 21

[Steve England [:stevee]]

Could this have caused bug 290469?

Comment 22

[Steve England [:stevee]]

Also, could this have caused bug 290464?

Comment 23

[Jerry Baker]

Is it also possible to have caused bug 290607 which regressed between 20050414
and 20050415?

Comment 24

[Gérard Talbot]

When I try attachment 180636 [details] from this bugfile, I get the actual results as
defined in bug 290597 with the provided steps to reproduce over there. That same
attachment 180636 [details] can be used to get the actual results as defined in bug 290553
with the provided steps to reproduce over there.

Mozilla 1.8b2 build 2005041606 under XP Pro SP2 here.

Comment 25

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Backing out this patch fixes the regressions in 290464 and elsewhere. 
nsViewManager changes don't affect things (though I notice that when the event
is handled, aEvent->point isn't restored as it was previously; not sure if that
will affect anything later on).  The nsBlockFrame change seems to have no
negative effects either, but the PresShell change seems to be the culprit. 
Backing just that chunk out fixes the 290464 behaviour, but the test cases here
are then broken.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Vlad, please don't reopen bugs unless:

1)  The patch has been backed out or
2)  The patch doesn't fix the bug that it claims to fix

If there are regressions, the work for them should be happening in the
regression bugs.  Not reopening the original bug reduces the amount of bugspam
people cced on its dependencies are forced to deal with and leads to less
confusion for QA trying to verify the fix.

Comment 27

[Peter van der Woude [:Peter6]]

If the long and still incomplete list of regressions caused by this patch aren't
going to be resolved before 1.8b2 (maybe an unrealistic goal) I suggest this
patch to be backed out.
Selection was a problem before this patch landed and has become a nightmare since.
We definitly wouldn't like to see this in the Developers Preview.

Comment 28

[WADA:World Anti-bad-Duping Agency]

Following are regression bug's timestamp of "marked FIXED".
 Bug 290464 2005-04-17 23:48 PDT
 Bug 290494 2005-04-17 23:48 PDT
 Bug 290522 2005-04-17 23:48 PDT
 Bug 290553 2005-04-18 14:06 PDT
These timestamp aren't exact check-in time of patch but check-in is around it.
Please be carefull when report mouse related bug on 4/17 and 4/18 builds.

Comment 29

[WADA:World Anti-bad-Duping Agency]

Additional patch check-in has been reported.
Bug 290469 has been marked as FIXED at 2005-04-19 16:06 PDT.

Comment 30

[Scott MacGregor]

could this have caused Bug #291537? I'm getting funky coordinates values for
event.clientX and event.clientY when trying to drag thunderbird local folders
around. 

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

mscott, is that related to any of the bugs marked as dependents here?