253076 - [FIX] clicking somewhere in the page does not remove selection

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Norbert]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a3) Gecko/20040724 Firefox/0.9.1+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a3) Gecko/20040724 Firefox/0.9.1+

Normally selected text can be deselected by just clicking somewhere within the
page. But I found some pages where this does not work!

So I have written and attached a simple html file to reproduce the problem.

Reproducible: Always
Steps to Reproduce:
1. open the attached html file
2. double-click the word "Test" in order to select it
3. click somewhere outside of the table: The selection is kept! This is the bug
I mean.
4. click somewhere within the table: The selection is correctly removed.



Expected Results:  
The selection should also be removed when clicking somewhere outside of the table.

(IE does not have such problems.)

Comment 1

[Norbert]

Attachment: HTML file to reproduce the described problem.

Comment 2

[Martin Creutziger [:MMx]]

I can confirm this bug on MacOS X 10.3.4 using recent Firefox *and* Mozilla builds.
Changing product to Browser, its not a Firefox-only bug.
can reproduce it with the following builds:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040720
Firefox/0.9.1+
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040720
Firefox/0.9.1+
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; de-AT; rv:1.7) Gecko/20040616

Comment 3

[Martin Creutziger [:MMx]]

sri for the bugspam, hope I picked the right component this time :|

Comment 4

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Forward BUTTON_DOWN events too.

This code actually suffers from the same problem as bug 180015, but I'm
guessing that Area(html) will never have a different view from Canvas(html)...
or can it?

Comment 5

[Mats Palmgren (inactive)]

Hmm, that didn't fix it actually...

Comment 6

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

This should do it.  This also solves the problem of starting a drag-select
from below the last line of text and dragging upwards, I think there is a bug
on that although I couldn't find it right now.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Mats, I have to admit to not knowing this code very well.  roc may be a better
reviewer than I here.  I'll try to figure it out tomorrow, but no guarantees...

Comment 8

[Mats Palmgren (inactive)]

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

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 154355 [details] [diff] [review]
Patch rev. 2

I'd not bet on the canvas and the area having the same view... especially if we
hack canvas some more.

I do have to wonder about this list of messages.  Don't we just want to forward
in all cases?  Or at least for aEvent->eventStructType == NS_MOUSE_EVENT ?

Also, why the change to blockframe?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 154355 [details] [diff] [review]
Patch rev. 2

r-, per comments

Comment 11

[Norbert]

I found that clicking on pictures also never removes selections. This can be
reproduced (for example) with the attached file "picture.htm".

Please make sure that also clicking on pictures always removes selections, like
it works in IE.

Comment 12

[Norbert]

Attachment: picture.htm

Comment 13

[Norbert]

(In reply to comment #11)

> Please make sure that also clicking on pictures always removes selections, like
> it works in IE.

I just found that IE only removes selections by clicking a picture, if the
picture is included in the selection. The same happens with Links: Clicking
links only removes selection if the link is included in the selection.


I'm not sure if IE's behaviour very logical...

I think clicking somewhere should always remove the selection independent of the
fact if the object is included in the selection or not.
What do you mean?

Comment 14

[chris hofmann]

if there is time to figure out reviews and get some testing on a patch in the
next few weeks then renominate.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 154355 [details] [diff] [review]
Patch rev. 2

clearing obsolete superreview request

Comment 16

[Nickolay_Ponomarev]

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

Comment 17

[Peter van der Woude [:Peter6]]

horrible bug when select has problems aswell

Comment 18

[Göran A]

(In reply to comment #17)
> horrible bug when select has problems aswell

I have also problem with select or more correct select and move.  When selecting
a text inside a box you should be able to "move" the text into another box and
the original box should become empty.  In Firefox the text remains in the
original fist text box.

Comment 19

[Eli Friedman]

The view stuff isn't an issue anymore, which is cool.  And I don't think there's any reason not to pass on all events to the selection code.

I'll post a patch in the next couple weeks.

Comment 20

[Eli Friedman]

Attachment: New Patch

It's nice to fix a bug by simply deleting a function.

This patch fixes this bug, bug 149267, and bug 324078, but not bug 290831.

Comment 21

[Eli Friedman]

Sorry for the spam, but I realized I didn't put in an explanation.

The function isn't needed anymore because the mouse selection code can deal with the canvas frame without any special casing.  Therefore, the generic implementation of HandleEvent in nsFrame is sufficient.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 209991 [details] [diff] [review]
New Patch

I like!

Comment 23

[Eli Friedman]

Checking in nsHTMLFrame.cpp;
/cvsroot/mozilla/layout/generic/nsHTMLFrame.cpp,v  <--  nsHTMLFrame.cpp
new revision: 1.156; previous revision: 1.155
done

Comment 24

[Martijn Wargers (dead)]

Well, the description of the bug in the "picture.htm" testcase is not fixed (clicking on the image should remove the selection)

Comment 25

[Eli Friedman]

Filed Bug 326500 on picture.htm, since it's really a separate issue.

Comment 26

[Simon Bünzli]

Does this simple fix also apply to the 1.8.1 branch? If so: any chance for getting it in there.

Comment 27

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

No, I think this depends on Eli's trunk changes.

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

How hard would it be to fix this on the branch without those changes?  And how common is the pattern of markup that triggers this bug?

Comment 29

[Eli Friedman]

A branch fix would be something along the lines of attachment 154355 [details] [diff] [review].  Mouse-down events on the canvas need to get forwarded so that the selection code gets to handle them.

The are basically two cases that cause this: pages with all the text in floats or positioned, and very short pages.

It's easy to work around by clicking on some text instead of the canvas.  It's more a polish issue than a usability issue.

Comment 30

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, given that, and that it would be a different patch than what's on the trunk (i.e., not yet tested), blocking1.8.1-.