Re: [g-a-devel]impl_getBoundedRanges

["Padraig O'Briain" <Padraig Obriain sun com>]

Bill,

I have looked at this more closely and have attached an updated patch.

I think this addresses the points you raised.

Padraig

> Subject: Re: [g-a-devel]impl_getBoundedRanges
> To: "Padraig O'Briain" <Padraig Obriain sun com>
> Cc: gnome-accessibility-devel gnome org
> Mime-Version: 1.0
> Content-Transfer-Encoding: 7bit
> 
> On Tue, 2003-01-14 at 14:40, Bill Haneman wrote:
> > On Tue, 2003-01-14 at 14:30, Padraig O'Briain wrote:
> > > Bill,
> > > 
> > > I have reviewed this patch and found that the change in the while 
statements 
> > > from < to <= is not necessary,
> > > 
> > > Updated patch is attached.
> > 
> > Thanks Padraig, this version of the patch looks good.  Please commit.
> ...
> 
> I took a second look, and I don't agree with this change:
> 
> > > -  startOffset = MIN (minLineStart, maxLineStart);
> > > -  endOffset  = MIN (minLineEnd, maxLineEnd);
> > > +  startOffset = MIN (minLineStart, minLineEnd);
> > > +  endOffset  = MAX (maxLineStart, maxLineEnd);
> 
> 
> Our API should guarantee that minLineStart <= minLineEnd, so your test
> is redundant.
> 
> However, my intent was to make sure that the line of text at (x, y) has
> a starting offset less than (or equal to) that of the line at (x+w,
> y+h).
> 
> Granted you need rather weird text (bottom-to-top alignment) for
> maxLineStart to be less than minLineStart in this example, but that was
> the intent.  So the patch should read, IMO:
> 
>  -  endOffset  = MIN (minLineEnd, maxLineEnd);
>  +  endOffset  = MAX (minLineStart, maxLineEnd);
> 
> FWIW,
> the only example I can think of is a "clock face" text buffer,
> containing the numbers 1 through 12 'in order', but laid out to look
> like this:
> 
>         12
>      11     1
>     10       2
>     9         3
>      8       4
>        7 6 5
> 
> 
> regards,
> 
> Bill
> 
> > >  
> > >    curr_offset = startOffset;
> > >  
> -- 
> Bill Haneman <bill haneman sun com>
> 

cvs server: Diffing .
Index: text.c
===================================================================
RCS file: /cvs/gnome/at-spi/libspi/text.c,v
retrieving revision 1.19
diff -u -p -r1.19 text.c
--- text.c	10 Jan 2003 14:45:53 -0000	1.19
+++ text.c	15 Jan 2003 11:48:17 -0000
@@ -559,6 +559,7 @@ impl_getBoundedRanges(PortableServer_Ser
   int curr_offset;
   gint minLineStart, minLineEnd, maxLineStart, maxLineEnd;
   long bounds_min_offset;
+  long bounds_max_offset;
 
   clip.x = x;
   clip.y = y;
@@ -568,14 +569,16 @@ impl_getBoundedRanges(PortableServer_Ser
   /* for horizontal text layouts, at least, the following check helps. */
   bounds_min_offset =  atk_text_get_offset_at_point (text, x, y, 
 						     (AtkCoordType) coordType);
+  bounds_max_offset =  atk_text_get_offset_at_point (text, x + width, y + height, 
+						     (AtkCoordType) coordType);
   atk_text_get_text_at_offset (text, bounds_min_offset, 
 			       ATK_TEXT_BOUNDARY_LINE_START,
 			       &minLineStart, &minLineEnd);
-  atk_text_get_text_at_offset (text, bounds_min_offset, 
+  atk_text_get_text_at_offset (text, bounds_max_offset, 
 			       ATK_TEXT_BOUNDARY_LINE_START,
 			       &maxLineStart, &maxLineEnd);
   startOffset = MIN (minLineStart, maxLineStart);
-  endOffset  = MIN (minLineEnd, maxLineEnd);
+  endOffset  = MAX (minLineEnd, maxLineEnd);
 
   curr_offset = startOffset;