Bug Hunting Session
Bug 43390 - Cannot cancel selections in writer using at-spi
Summary: Cannot cancel selections in writer using at-spi
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other Linux (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-30 12:31 UTC by Vincent Povirk
Modified: 2011-12-19 06:04 UTC (History)
0 users

See Also:
Crash report or crash signature:


Attachments
hacky patch that makes things work for me (3.16 KB, patch)
2011-11-30 14:34 UTC, Vincent Povirk
Details
Proper patch (1.63 KB, patch)
2011-12-16 12:55 UTC, Vincent Povirk
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Povirk 2011-11-30 12:31:08 UTC
Once something is selected in Writer, it's impossible to get Writer into a normal state with nothing selected using at-spi. While calling RemoveSelection(0) for a paragraph appears to work, it actually leaves writer in a state where moving the caret, through keyboard/mouse input or SetCaretOffset, will select a range from the beginning of the line to the new caret offset. SetCaretOffset on its own does not cancel selections (I'm not sure whether it should or not.).

I have a patch for this, but I think I need to understand things a bit better before I can send it.
Comment 1 Vincent Povirk 2011-11-30 14:34:00 UTC
Created attachment 53986 [details]
hacky patch that makes things work for me

This is probably wrong and not in any shape to be committed (and I didn't even remove my debug prints), but I want to have it here for completeness. I suspect my change to setSelection here doesn't even work, and the change to setCaretPosition is just hiding that problem.

I think the real trouble here is that Select() doesn't work properly when given a PaM with no mark, if a mark already exists in the document.

A secondary problem is that setSelection() with an empty range (which is how atk's RemoveSelection is implemented) calls Select() with a PaM that has a mark. It should probably behave like setCaretPosition in this case.

I'd really like to understand why Select() is broken before continuing.
Comment 2 Vincent Povirk 2011-12-16 12:55:23 UTC
Created attachment 54515 [details]
Proper patch

So it turns out the selection weirdness I'm seeing in Writer with accessibility api's has several different causes:

SwWrtShell uses the SttSelect and EndSelect methods to decide when to extend the selection as the user moves the cursor, and when to kill the selection. The upshot of this is that if we set the selection without calling SttCursor (which the accessibility code does), SwWrtShell does not know to kill the selection when the user later moves the cursor, and instead the selection is extended. This causes bad behavior when something is selected through the accessibility API and the user later moves the cursor manually.

The "RemoveSelection" method is implemented by doing SetSelection(0, 0), creating a selection that starts and ends at the start of a line. While this looks the same as removing the selection, it's actually subtly different and causes a problem later.

The "SetCaretOffset" method, unlike "RemoveSelection", selects a PaM without a mark. That means that if there's currently a selection, even an empty selection of the sort that "RemoveSelection" creates, it extends that selection. I think this is OK (I remember reading somewhere that moving the caret has an undefined result if there's an existing selection), but if we call "RemoveSelection" followed by "SetCaretOffset" then that should result in no selection.

I've chosen to address the first two issues in my patch and leave the third one alone. We could make SetCaretOffset kill the selection, but I think the way it behaves now is also a correct behavior, as long as RemoveSelection is fixed.
Comment 3 Cédric Bosdonnat 2011-12-19 06:04:15 UTC
Patch pushed in both master and 3.5 branches.