801421 - Selection's end point is not updated after the range is changed

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: Text Selection, defect)

Description

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: simple html page to reproduce the issue

I'm working on makes our selection's module in Add-on SDK (jetpack) working on Fennec.

I found that when we replace the current selection's text, the end point of the selection is not updated on Fennec, even if text is selected properly.

See the screenshot for more information, and the simple HTML page attached to reproduce the behavior.

Tested on Samsung Galaxy Tab.

Comment 1

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: the screenshot of the issue

The end point of selection in the screenshot, is still pointing to the previous selection area.

Comment 2

[Aaron Train [:aaronmt]]

Which build are you using?

Comment 3

[Matteo Ferretti [:zer0] [:matteo]]

Firefox release (16.0) and Nightly (19.0.1), but this one is not the latest one, because the latest one constantly crashes.

Comment 4

[Matteo Ferretti [:zer0] [:matteo]]

I wipe out the profile and tested with the latest Nightly, the issue is still there.

Comment 5

[:Margaret Leibovic]

It looks like we need to add some code to update the handle positions if the selection changes. We're already listening for selection changes to check to see if the selection is collapsed, so we can stick logic to fix this issue in there:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1727

I think a call to updateCacheForSelection() and positionHandles() in there should do the job. However, we need to check to see if notifySelectionChanged() is called when we change the selection ourselves in moveSelection, because the Java side of things we already be handling repositioning the handles.

I'm marking this a mentor bug, but I don't know that it would make a good first bug, because this could be tricky.

Comment 6

[:Margaret Leibovic]

This code has changed with bug 667243, so my last comment is no longer valid.

This could probably be fixed as part of bug 864589.

Comment 7

[Mark Capella [:capella]]

Hmmmm ... range level changes (range.deleteContents(); range.insertNode(node); range.selectNode(node);) don't trigger parent level selectionListener notifications that we can intercept and act on.

Modifying the example code like: http://jsfiddle.net/nxAtn/2/ produces a cleaner result as a workaround ...

Comment 8

[Mark Capella [:capella]]

meh ... my mistake, that removes the selection entirely, in addition to the handles

Comment 9

[Mark Capella [:capella]]

I've had this bug in mind as possibly being solved using work that bug 1060579 may provide (enhanced selectionListeners)

However, on re-testing the original issue to verify, it appears to me that our work on Bug 895463 has already fixed it!

If I can have Q/A verify it, I think we can close this.

Comment 10

[Mark Capella [:capella]]

meh ... should've just ni'd in the first place