459921 - Double-click-and-drag selects previous word when dragging to left half of first letter of a word

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[Uri Bernstein]

When dragging after double-click (selecting in word-by-word mode), if you drag to the first (left) half of the first letter of a word, the previous word gets selected as well.

This is similar to bug 200098, and should be fixable in a similar manner.

Comment 1

[Uri Bernstein]

Attachment: testcase

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Note:  Keep the mouse down after you double-click on "Web" in the testcase.

Comment 3

[Uri Bernstein]

Attachment: patch

Similar to bug 200098.

Comment 4

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

Comment on attachment 343828 [details] [diff] [review]
patch

we really need tests for this stuff

Comment 5

[Uri Bernstein]

Attachment: mochitest

I'll wait until someone verifies that my test for bug 415707 works on Windows/Linux before checking this in (unless someone wants to verify this test, which would be nice).

Comment 6

[Uri Bernstein]

Comment on attachment 343983 [details] [diff] [review]
mochitest

Actually, Roc, would you like to have a look at the test first?

Comment 7

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

Using synthesizeMouse would take care of the accelerator-key selection and getBoundingClientRect for you. Otherwise looks good.

Comment 8

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

That's in EventUtils.js

Comment 9

[Uri Bernstein]

Attachment: better mochitest

Using EventUtils.js, much nicer.

Comment 10

[Uri Bernstein]

Attachment: better mochitest

The real test, this time.

Comment 11

[Uri Bernstein]

http://hg.mozilla.org/mozilla-central/rev/edd4e55b65fe

Comment 12

[Uri Bernstein]

I backed out the mochitest since it triggered bug 426195, and the workaround we used last time (do another click at the end) only fixed Mac, and broke a different test on Linux (that wasn't affected initially).

Comment 13

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

>     nsIFrame* frame = GetFrameForNodeOffset(offsets.content, offsets.offset, HINTRIGHT, &offset);
>+
>+    if (amount == eSelectWord && direction == eDirPrevious) {
>+      // To avoid selecting the previous word when at start of word,
>+      // first move one character forward.
>+      nsPeekOffsetStruct charPos;
>+      charPos.SetData(eSelectCharacter, eDirNext, offset, 0,
>+                      PR_FALSE, mLimiter != nsnull, PR_FALSE, PR_FALSE);
>+      if (NS_SUCCEEDED(frame->PeekOffset(&charPos))) {

^ frame should be null checked here (or perhaps in the if() block above), like it is when it's used below. 

>+        frame = charPos.mResultFrame;
>+        offset = charPos.mContentOffset;
>+      }
>+    }

I think this is causing my crash at http://crash-stats.mozilla.com/report/index/a795e040-a45f-11dd-b7a9-001cc4e2bf68?p=1 -- caused by selecting and deselecting some stuff on a bugzilla bug list page.

Comment 14

[Aakash Desai [:aakashd]]

verified FIXED on builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090413 Minefield/3.6a1pre ID:20090413031052

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090413 Minefield/3.6a1pre ID:20090413031052

Comment 15

[Aakash Desai [:aakashd]]

ack, here's the build ID for Shiretoko Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090413 Shiretoko/3.5b4pre ID:20090413031313