The flows-from flows-to relations should be present.

Apologies for spam-- ensuring Sun a11y team are cc'ed on all current a11y bugs.
 Filter on "SUN A11Y SPAM" to ignore.

Created attachment 37489 [details] [review]
proposed patch

Comment on attachment 37489 [details] [review]
proposed patch

Please use ALLCAPS for this macro, or rename it.  Using the naming convention
you use below, it would be likely to conflict with a 'real'
AccessibleRelationSet_free() API if we add one in the future (and we probably
should).


>+#define AccessibleRelationSet_free(rel)			\
>+    {							\
>+	gint k;						\
>+	srl_assert (rel);				\
>+	for (k = 0; rel[k]; k++)			\
>+	    AccessibleRelation_unref (rel[k]);		\
>+	g_free (rel);					\
>+    }

That was the ideea. When that function will be added in at-spi, the macro will
be removed without other changes.

Remus: that would make gnopernicus depend _forwards and backwards_ on specific
versions of at-spi, which is not OK.

gnopernicus will depend forwards on a specific at-spi version when the new
function will be added.

Created attachment 39059 [details] [review]
reworked patch

Comment on attachment 39059 [details] [review]
reworked patch

>Index: gnopernicus/srlow/libsrlow/SRObject.c
...
>+	if (cnt > 0)
>+	{
>+	    gchar *str = NULL;
>+	    long int sp, ep;
>+	    AccessibleText_getSelection (text, cnt - 1, &sp, &ep);
>+	    str = AccessibleText_getText (text,  sp, ep);
>+	    g_string_insert (sel, 0, " ");
>+	    g_string_insert (sel, 0, str ? str : "");
>+	    SPI_freeString (str); 
>+	}

This seems to assume that the last selection from the FLOWS_FROM object is
contiguous with the current object's selection(s).  This isn't always true...
...

>+	    long int sp, ep;
>+	    AccessibleText_getSelection (text, 0, &sp, &ep);
>+	    str = AccessibleText_getText (text,  sp, ep);
>+	    g_string_append (sel, " ");
>+	    g_string_append (sel, str ? str : "");

Same issue arised here, I think.

...

>+	if (n_sel > 1)
>+	{
>+	    long i;
>+	    for (i = 0; i < n_sel; i++)
>+	    {
>+		gchar *tmp;
>+		long int start, end;
>+		AccessibleText_getSelection (acc_text, i, &start, &end);
>+		tmp = AccessibleText_getText (acc_text, start, end);
>+		(*selections)[i] = SR_strdup (tmp);
>+		SPI_freeString (tmp);
>+	    }
>+	}

Does this mean that selections from FLOWS relations are only appended if there
is only one selection in the object?

- Bill

>This seems to assume that the last selection from the FLOWS_FROM object is
>contiguous with the current object's selection(s).  This isn't always true...

For a multiple selection, like:
  1111_____222222 (first object)
  cccccC______ (second object, has FLOWS_FROM relation with first object)
  (1= a selected char from first selection, 2=a selected char form second
selection, c=a selected char from current object, C=the caret)

In this case, gnopernicus has to report "222222\nccccc" (this is _current_
selection). So, from the objects with FLOWS_FROM relation, _only_ the last
selection is part of current (for user) selection.

Same observation for first selection in case of FLOWS_TO relation.

>Does this mean that selections from FLOWS relations are only appended if there
>is only one selection in the object?
Yes, I did not see (yet) any case where is possible to have multiple selections
and FLOWS_TO/FROM relations.

This isn't right: the 'current selection' should in fact be "the current
selection set".  I still think your patch is incorrect because it doesn't ensure
that the selected text from the previous object (FLOWS_FROM) is contiguous with
the selected text in the current object. 

Bill, is your case a practical one? Or is just in theory?

I agree your scenario is possible, but I don't think it is practical.

My case will occur with any application or text widget that supports
multi-selection.

Do they all have FLOWS_TO/FLOWS_FROM relations?

The ideal case for text widgets is that text is contained in multiple, sibling
objects, many of which have the FLOWS_FROM/FLOWS_TO relation.  In any such
widget or application which supports multiple selection, your patch will fail to
work correctly.

It isn't terrible important whether such "ideal cases" are common or not, as
they are the implementations which we are striving for.  If our algorithms break
if and when applications decide to do things "right", then our algorithms are wrong.

Created attachment 45293 [details] [review]
reworked patch

The patch doesn't work well because of bug #301040.

This bug depends on bug #301040.

I tested the patch with StarOffice (which has FLOWS_TO/FLOWS_FROM relations) and it works fine.

Patch looks good to me.  Since it works with FLOWs stuff in OOo 2.0.1 build m155, please apply and close this bug.  Please also note in the appropriate OOo bug (I don't have the number to hand) the version of Gnopernicus that this patch is contained in so they can "close the loop" with their bug on their end.  Thanks!

Patch committed to CVS head.