Closed Bug 253076 Opened 16 years ago Closed 14 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: norbert.notz, Assigned: sharparrow1)

References

(Blocks 1 open bug)

Details

(Keywords: testcase, Whiteboard: [no l10n impact])

Attachments

(3 files, 2 obsolete files)

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.)
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
Assignee: firefox → nobody
Severity: major → normal
Status: UNCONFIRMED → NEW
Component: General → Layout: Tables
Ever confirmed: true
Flags: blocking-aviary1.0?
OS: Windows 2000 → All
Product: Firefox → Browser
QA Contact: firefox.general → core.layout.tables
Hardware: PC → All
Version: unspecified → Trunk
sri for the bugspam, hope I picked the right component this time :|
Assignee: nobody → events
Component: Layout: Tables → Event Handling
QA Contact: core.layout.tables → ian
Assignee: events → mats.palmgren
Keywords: testcase
Summary: clicking somewhere in the page does not remove selection → [FIX] clicking somewhere in the page does not remove selection
Attached patch Patch rev. 1 (obsolete) — Splinter Review
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?
Attachment #154352 - Attachment is obsolete: true
Hmm, that didn't fix it actually...
Summary: [FIX] clicking somewhere in the page does not remove selection → clicking somewhere in the page does not remove selection
Attached patch Patch rev. 2 (obsolete) — Splinter Review
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.
Summary: clicking somewhere in the page does not remove selection → [FIX] clicking somewhere in the page does not remove selection
Attachment #154355 - Flags: superreview?(dbaron)
Attachment #154355 - Flags: review?(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...
Blocks: 149267
*** Bug 253152 has been marked as a duplicate of this bug. ***
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 on attachment 154355 [details] [diff] [review]
Patch rev. 2

r-, per comments
Attachment #154355 - Flags: review?(bzbarsky) → review-
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.
Attached file picture.htm
(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?
if there is time to figure out reviews and get some testing on a patch in the
next few weeks then renominate.
Flags: blocking-aviary1.0? → blocking-aviary1.0-
Comment on attachment 154355 [details] [diff] [review]
Patch rev. 2

clearing obsolete superreview request
Attachment #154355 - Flags: superreview?(dbaron)
*** Bug 290497 has been marked as a duplicate of this bug. ***
horrible bug when select has problems aswell
Flags: blocking1.8b2?
Flags: blocking-aviary1.1?
Blocks: 290831
Flags: blocking1.8b2?
Flags: blocking1.8b2-
Flags: blocking-aviary1.0-
Flags: blocking1.8b3?
Flags: blocking1.8b4?
Flags: blocking1.8b3?
Flags: blocking1.8b3-
Flags: blocking1.8b2-
(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.
Whiteboard: [no l10n impact]
Flags: blocking-aviary1.1?
Flags: blocking1.8b4? → blocking1.8b4-
Flags: blocking1.8.1?
Flags: blocking1.9a1?
Blocks: 324078
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.
Assignee: mats.palmgren → sharparrow1
Attached patch New PatchSplinter Review
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.
Attachment #154355 - Attachment is obsolete: true
Attachment #209991 - Flags: review?(bzbarsky)
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 on attachment 209991 [details] [diff] [review]
New Patch

I like!
Attachment #209991 - Flags: review?(bzbarsky) → review+
Attachment #209991 - Flags: superreview?(roc)
Attachment #209991 - Flags: superreview?(roc) → superreview+
Checking in nsHTMLFrame.cpp;
/cvsroot/mozilla/layout/generic/nsHTMLFrame.cpp,v  <--  nsHTMLFrame.cpp
new revision: 1.156; previous revision: 1.155
done
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Well, the description of the bug in the "picture.htm" testcase is not fixed (clicking on the image should remove the selection)
Filed Bug 326500 on picture.htm, since it's really a separate issue.
Flags: blocking1.9a1?
Does this simple fix also apply to the 1.8.1 branch? If so: any chance for getting it in there.
Flags: blocking1.8.1?
No, I think this depends on Eli's trunk changes.
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?
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.
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-.
Flags: blocking1.8.1? → blocking1.8.1-
Depends on: 349931
Depends on: 370174
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.