263099 - Double-clicking a "margin" often selects a strange bit of text elsewhere on the page

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[Trogdor]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10

Go to http://en.wikipedia.org/wiki/United_States_Constitution#Preamble which is
a Wikipedia article.  (I am assuming you don't use any Wikipedia skins.)  Try
doubleclicking in the 'margin' on the left.  Depending on where you click
horizontally, different words 

Reproducible: Always
Steps to Reproduce:
1.  Go to http://en.wikipedia.org/wiki/United_States_Constitution#Preamble
2.  Move the mouse so it's to the left of the 2nd line of the paragraph about
the Preamble.  Double-click.
3.  Note that a word in the 4th line in the paragraph about the Preamble will be
highlighted.  (strange behavior)
4.  Try moving the mouse to the right or left in the 'margin' (such that the
mouse is still to the left of the 2nd line in the Preamble paragraph).  Repeat
step #3.  Note that a different word is highlighted.

Actual Results:  
Various words get highlighted that I wouldn't expect to get highlighted by this
action.

Expected Results:  
Probably highlight nothing, since I was not doubleclicking on any text.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Minimal testcase needed...

Comment 2

[Martijn Wargers (dead)]

The patch for bug 180015 seems to fix this mostly.
The horizontal selection offset is gone with that patch. 
But the small vertical selection offset (selection 2 lines lower than where
you've activated it with the mouse)is still there with the patch.

Comment 3

[Mats Palmgren (inactive)]

I also see the 2-line vertical offset problem on Linux with bug 180015 fixed.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Martijn, could you minimize that testcase, by any chance?

Comment 5

[Martijn Wargers (dead)]

Attachment: Minimised testcase

Sure. The position:relative is needed and the extra nested <div> is needed for
the bug to happen.

Comment 6

[Boris Zbarsky [:bzbarsky]]

I'm betting that we're messing up the coordinate transformation (because the
nearest view to the click point is not the view of the rel pos element or
something like that).

Comment 7

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

Attachment: fix

nsBlockFrame::HandleEvent drills down to find a block where selection should be
happening. As predicted, it's assuming that the view for the child block is the
same as the view for the current block (to which aEvent->point is relative) ---
obviously incorrect. So this code uses frame->GetOffsetTo to get the delta
between the child block and the current block and adds that to the offset from
the current block to its containing view.

Comment 8

[Boris Zbarsky [:bzbarsky]]

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

So... could you fix the comment on GetClosestLine() to make sense?  Its
documentation for aOrigin tells me nothing.

I can't even tell whether this patch is correct, because I can't tell what this
code is really doing.  :(

Comment 9

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

Attachment: fix #2

OK, I removed the stupid aOrigin parameter and I'm just forcing aPoint to be
relative to the line iterator's block's coordinate system.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173572 [details] [diff] [review]
fix #2

Much better, thank you!

r+sr=bzbarsky

Comment 11

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

checked in.