210197 - Mouse selection get stuck after selecting table elements (CTRL+click)

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[Mats Palmgren (inactive)]

Mouse selection get stuck after selecting table elements (CTRL+click)

REPEAT BY
1. go to http://www.cnn.com/  (or any page with a table)
2. CTRL+click somewhere in the left column but not on any link text,
   (the navbar: "World" "U.S." etc) and then drag the mouse up and
   out of the window still holding CTRL down
3. try to deselect the cells by clicking on the background or by
   drag-selecting a chunk of text using the mouse
   

ACTUAL RESULTS
Table cells are marked as selected as I drag the mouse upwards, but after
releasing the mouse button (de)selection using the mouse stops working.
"Select all" using CTRL+A and deselecting with ESC works fine though,
but mouse selection is still stuck after that.

EXPECTED RESULT
Working mouse (de)selection

PLATFORM & VERSIONS TESTED
Bug occurs in Mozilla nightly trunk build 2003-06-20-08 on Linux
Bug occurs in Mozilla 1.3.1 on Linux
Bug occurs in Mozilla 1.0.2 on Linux

ADDITIONAL INFORMATION
Other windows and tabs works fine so it's not a hang.
A Reload cures the problem.

Comment 1

[Ben Goodger (use ben at mozilla dot org for email)]

Yeah this isn't going to get fixed if it stays assigned to mjudge. 

Looking for a new owner... 

Comment 2

[Ben Goodger (use ben at mozilla dot org for email)]

I see this on windows too. 

Comment 3

[Henrique Rodrigues]

i've noticed this too under linux and windows, with mozilla 1.6b and firebird 0.7.

to replicate the problem correctly you must relese CTRL before the mouse button
when selecting the table cells.

Comment 4

[Wira Mulia]

Confirmed in Linux with Mozilla 2004-04-20 1.7 branch nightly.

Comment 5

[Prognathous]

*** Bug 254663 has been marked as a duplicate of this bug. ***

Comment 6

[Anders Conbere]

*** Bug 294843 has been marked as a duplicate of this bug. ***

Comment 7

[Kathleen :Brade]

I'm not a selection guru by any means.  However, a little poking around with lxr
seems to point to nsSelection::HandleTableSelection().
Here is a snippet of code (with lxr's line numbers):

3310       // Any other mouseup actions require that Ctrl or Cmd key is pressed
3311       //  else stop table selection mode
3312       PRBool doMouseUpAction = PR_FALSE;
3313 #if defined(XP_MAC) || defined(XP_MACOSX)
3314       doMouseUpAction = aMouseEvent->isMeta;
3315 #else
3316       doMouseUpAction = aMouseEvent->isControl;
3317 #endif
3318       if (!doMouseUpAction)
3319       {
3320 #ifdef DEBUG_TABLE_SELECTION
3321 printf("HandleTableSelection: Ending cell selection on mouseup:
mAppendStartSelectedCell=%d\n", mAppendStartSelectedCell);
3322 #endif
3323         return NS_OK;
3324       }

There needs to be a different check; checking whether the control key is pressed
on the mouse up event is insufficient (as reported in this bug).  Perhaps this
block of code can be removed?

cc'ing jst and peterv in case one of them has a few cycles to look at this
problem (or can point someone else who does to this bug).
Hopefully someone else can pick up from this spot and fix this bug.

Comment 8

[Uri Bernstein]

Attachment: patch v1

The real issue here is that mDragSelectingCells is not being reset when the mouse-up even occurs outside the table.
This fix makes the assumption that when MouseDown is set to false, dragging is necessarily finished, so mDragSelectingCells could be reset.

A more conservative (and less elegant) fix would be to add a setter for mDragSelectingCells, and call it explicitly from HandleFrameSelection() in the !aParentContentForTableSel case.

Comment 9

[Uri Bernstein]

This bugs affects contenteditable frames pretty badly, and thus hurts webapps allowing rich text editing. Fixing it for 1.9.1 would really help. 

Comment 10

[Uri Bernstein]

Comment on attachment 329224 [details] [diff] [review]
patch v1

Reassigning to Mats per dbaron's recommendation. Mats - could you please have a look at this in time for 1.9.1?

Comment 11

[Mats Palmgren (inactive)]

Comment on attachment 329224 [details] [diff] [review]
patch v1

Looks good.  r+sr=mats

Comment 12

[Uri Bernstein]

Checked in to mozilla-central.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified on trunk and 1.9.1 with following builds on OS X and Windows:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204020327

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/2008080102 Minefield/3.1a2pre

Comment 14

[Ricardo Maçãs [:Ricmacas]]

I suspect this bug fix to be causing https://bugzilla.mozilla.org/show_bug.cgi?id=210197 and all the dupes and etc behind it.

Comment 15

[Ricardo Maçãs [:Ricmacas]]

I meant https://bugzilla.mozilla.org/show_bug.cgi?id=474458 , sorry.