506874 - reducing selection with mouse or keyboard instead collapses selection highlight

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[skierpage]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090727 Minefield/3.6a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090727 Minefield/3.6a1pre

In today's nightly build, using shift + arrow keys to reduce the selection makes the selection highlight go away and otherwise behave incorrectly in the location field, text boxes and textareas.  It's easiest to see it by making a selection to the left, then reducing it, but it's messed up in other situations.

Reproducible: Always

Steps to Reproduce:
1.  Press Ctrl+L to go to the location field, then End to set the insertion point to the end.
2.  Press Shift+Left arrow a few times to extend the selection to the left
3.  Press Shift+Right arrow.
Actual Results:  
The entire selection highlight goes away.

Expected Results:  
The selection highlight should reduce by one.

The actual selection is correct as you can see if you copy it, it's just the visual display of the selection highlight that's wrong.  It's as if it can only grow, it can't shrink.

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

i confirm with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090727 Minefield/3.6a1pre ID:20090727044731

Comment 2

[skierpage]

I have the same issue with mouse selection (the selection highlight on a line
vanishes when you shrink the selection), so this is more a rendering issue
than keyboard navigation. I picked a different component.

Comment 3

[Alice0775 White]

The regression range is:
Works Fine:
http://hg.mozilla.org/mozilla-central/rev/5e62c879c8d5
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090726 Minefield/3.6a1pre ID:20090726184302

Broken:
http://hg.mozilla.org/mozilla-central/rev/3b37889fa8a4
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090726 Minefield/3.6a1pre ID:20090726193509

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5e62c879c8d5&tochange=3b37889fa8a4

Comment 4

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

Attachment: fix

The main fix here is in nsTypedSelection::Extend, removing the code which temporarily removes mAnchorFocusRange from the selection while we call selectFrames. This is no longer needed and in fact breaks the new selection code in nsTextFrame.

There are tons of tests testing all the combinations of old-focus-point, anchor-point and new-focus-point in a text frame and in a list of images. While writing the tests I found a few other bugs:

-- setting up a selection while the page is loading can fail to draw the selection, because we're not flushing frame construction anywhere so we fail to find frames to set the selection bits on. I'm adding a Flush_Frames to fix that.

-- setting up a selection on images while the page is loading can fail to draw the selection, because the images get reframed when they load which causes our selection bits to be discarded. The right fix for this is to move selection to content, but I'm not doing that for now. Instead I'm working around it in the image tests by waiting for load to fire before setting up the selection.

-- When nsTypedSelection::Extend is called with the new focus point set to the current focus point, it returns a failure code. That is dumb, it should just succeed and do nothing.

Comment 7

[Daniel Holbert [:dholbert]]

FWIW, this regressed between these nightlies:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090726 Minefield/3.6a1pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090727 Minefield/3.6a1pre

Changelog for regression:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=80eefd4207ce&tochange=0322169bc93e

bug 371839 ("Simplify SetSelected signature and semantics, and reimplement it in nsTextFrame much more efficiently.") looks highly suspicious. Marking as blocking that bug; if that's incorrect, feel free to update with correct info.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

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


>+    presShell->FlushPendingNotifications(Flush_Frames);
>+
This may kill presshell, so when GetShell() is used later in the method,
there might be some null pointer crashes.

With that fixed somehow, r=me.

Comment 9

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

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

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090803 Minefield/3.6a1pre.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Mass change: adding fixed1.9.2 keyword

(This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)

Comment 12

[Tracy Walker [:tracy]]

verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a2pre) Gecko/20090911 Namoroka/3.6a2pre and equivalent Mac build