Closed Bug 506874 Opened 11 years ago Closed 10 years ago

reducing selection with mouse or keyboard instead collapses selection highlight

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: info, Assigned: roc)

References

Details

(Keywords: regression, verified1.9.2)

Attachments

(1 file)

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.
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
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.
Component: Keyboard: Navigation → GFX: Thebes
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
Assignee: nobody → roc
Component: GFX: Thebes → Layout
Flags: blocking1.9.2+
QA Contact: keyboard.navigation → layout
Summary: reducing selection with Shift+arrow instead collapses selection highlight → reducing selection with mouse or keyboard instead collapses selection highlight
Attached patch fixSplinter Review
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.
Attachment #391271 - Flags: review?(Olli.Pettay)
Duplicate of this bug: 507311
Duplicate of this bug: 507333
Whiteboard: [needs review]
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.
Blocks: 371839
OS: Windows XP → All
Hardware: x86 → All
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.
Attachment #391271 - Flags: review?(Olli.Pettay) → review+
Whiteboard: [needs review] → [needs landing]
http://hg.mozilla.org/mozilla-central/rev/b914eaf5d20e
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
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.
Status: RESOLVED → VERIFIED
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)
Keywords: fixed1.9.2
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
Keywords: verified1.9.2
Depends on: 527306
You need to log in before you can comment on or make changes to this bug.