*** empty log message ***
[platform/core/uifw/at-spi2-atk.git] / test / screen-review-test.c
index b0cee5f..abc916d 100644 (file)
@@ -2,7 +2,8 @@
  * AT-SPI - Assistive Technology Service Provider Interface
  * (Gnome Accessibility Project; http://developer.gnome.org/projects/gap)
  *
- * Copyright 2001 Sun Microsystems Inc.
+ * Copyright 2001, 2002 Sun Microsystems Inc.,
+ * Copyright 2001, 2002 Ximian, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -20,6 +21,8 @@
  * Boston, MA 02111-1307, USA.
  */
 
+#include <stdio.h>
+#include <strings.h>
 #include <stdlib.h>
 #include "../cspi/spi-private.h"
 
  *
  * Issues:
  *
- * * bounds now fail to include window border decoration
- *         (which we can't really know about).
+ * * there are bugs in the compositing code.
  *
  * * brute-force algorithm uses no client-side cache; performance mediocre.
  *
  * * we don't have good ordering information for the toplevel windows,
  *          and our current heuristic is not guaranteed if
- *          the active window is not always on top.
+ *          the active window is not always on top (i.e. autoraise is
+ *          not enabled).
  *
- * * text-bearing objects that don't implement AccessibleText (such as buttons)
- *          don't get their text clipped since we don't know the character
- *          bounding boxes.
  * * can't know about "inaccessible" objects that may be obscuring the
  *          accessible windows (inherent to API-based approach).
  *
+ * * text column alignment is crude since it relies on a hard-coded
+ *          (and arbitrary) ratio of horizontal pixels to character columns.
+ *          For small proportional fonts, overruns are likely to result in
+ *          occasional review lines that exceed the standard length.
+ *
  * * (others).
  */
 
+#undef CHUNK_LIST_DEBUG /* define to get list of text chunks before clip */
 
-#define BOUNDS_CONTAIN_X(b, p)  (((p)>=(b)->x) && ((p)<=((b)->x + (b)->width))\
-           && ((b)->width > 0) && ((b)->height > 0))
+#define BOUNDS_CONTAIN_X_BOUNDS(b, p)  ( ((p).x>=(b).x) &&\
+                                        (((p).x + (p).width) <= \
+                                         ((b).x + (b).width)) && \
+                                       ((b).width > 0) && ((b).height > 0))
 
 #define BOUNDS_CONTAIN_Y(b, p)  (((p)>=(b)->y) && ((p)<=((b)->y + (b)->height))\
            && ((b)->width > 0) && ((b)->height > 0))
@@ -97,7 +105,7 @@ typedef struct _BoundaryRect {
 
 typedef struct _TextChunk {
        char           *string;
-       AccessibleText *source;
+       Accessible     *source;
        int             start_offset;
        int             end_offset;
        BoundaryRect    clip_bounds;
@@ -106,7 +114,7 @@ typedef struct _TextChunk {
        BoundaryRect    end_char_bounds;
 } TextChunk;
 
-typedef struct _ScreenReviewBuffer { /* TODO: implement */
+typedef struct _ScreenReviewBuffer { 
        GList *text_chunks;
 } ScreenReviewBuffer;
 
@@ -175,7 +183,7 @@ clip_bounds_clone (BoundaryRect *bounds[])
        BoundaryRect **bounds_clone;
        bounds_clone = (BoundaryRect **)
                g_new0 (gpointer, SPI_LAYER_LAST_DEFINED);
-       for (i = 0; i < SPI_LAYER_LAST_DEFINED; ++i) {
+       for (i = 1; i < SPI_LAYER_LAST_DEFINED; ++i) {
                bounds_clone[i] = g_new0 (BoundaryRect, 1);
                *bounds_clone[i] = *bounds[i];
        }
@@ -216,7 +224,7 @@ boundary_xclip_head (BoundaryRect *bounds, BoundaryRect *clipBounds)
        int cx2 = clipBounds->x + clipBounds->width;
        if (cx2 < bounds->x) return;
        x2 = bounds->x + bounds->width;
-       if (cx2 < x2) bounds->x = cx2;
+       if (cx2 <= x2) bounds->x = cx2;
        bounds->width = MAX (0, x2 - cx2);
 }
 
@@ -234,7 +242,7 @@ text_chunk_copy (TextChunk *chunk)
        TextChunk *copy = g_new0 (TextChunk, 1);
        *copy = *chunk;
        if (chunk->string) copy->string = g_strdup (chunk->string);
-       if (copy->source) AccessibleText_ref (copy->source);
+       if (copy->source) Accessible_ref (copy->source);
        return copy;
 }
 
@@ -300,7 +308,7 @@ text_chunk_split_insert (GList *chunk_list, GList *iter, TextChunk *chunk)
 
 /* #define PRINT_CHUNK_DEBUG(a, b, c, d) print_chunk_debug(a, b, c, d) */
 
-#define PRINT_CHUNK_DEBUG(a, b, c, d) 
+#define PRINT_CHUNK_DEBUG(a, b, c, d)
 
 #ifdef PRINT_CHUNK_DEBUG
 static void
@@ -323,12 +331,11 @@ text_chunk_list_head_clip (GList *text_chunk_list,
 {
        GList *target, *iter = next, *prev;
        prev = iter->prev;
-//     if (chunk->string && strlen (chunk->string)) { 
+/*     if (chunk->string && strlen (chunk->string)) { */
                text_chunk_list =
                        g_list_insert_before (text_chunk_list, next, chunk);
-//     }
-       while (iter) {
-               if (CHUNK_BOUNDS_SPANS_END (chunk, (TextChunk *)iter->data)) {
+/*     }*/
+       while (iter && CHUNK_BOUNDS_SPANS_END (chunk, (TextChunk *)iter->data)) {
 #ifdef CLIP_DEBUG                      
                        fprintf (stderr, "deleting %s\n",
                                 ((TextChunk *)iter->data)->string);
@@ -337,22 +344,19 @@ text_chunk_list_head_clip (GList *text_chunk_list,
                        iter = iter->next;
                        text_chunk_list =
                                g_list_delete_link (text_chunk_list, target);
-               } else {
-                       if (!CHUNK_BOUNDS_END_BEFORE_START (chunk,
-                                                     (TextChunk *)iter->data)) {
-                               text_chunk_head_clip ((TextChunk *)iter->data,
-                                                     chunk);
-                       }
-                       if (prev &&
-                           !CHUNK_BOUNDS_AFTER_END (
-                                   chunk,
-                                   (TextChunk *)prev->data)) {
-                               text_chunk_tail_clip (
-                                       (TextChunk *)prev->data,
-                                       chunk);
-                       }
-                       break;
-               }
+       }
+       if (iter && !CHUNK_BOUNDS_END_BEFORE_START (chunk,
+                                                   (TextChunk *)iter->data)) {
+               text_chunk_head_clip ((TextChunk *)iter->data,
+                                     chunk);
+       }
+       if (prev &&
+           !CHUNK_BOUNDS_AFTER_END (
+                   chunk,
+                   (TextChunk *)prev->data)) {
+               text_chunk_tail_clip (
+                       (TextChunk *)prev->data,
+                       chunk);
        }
        
        return text_chunk_list;
@@ -365,21 +369,21 @@ text_chunk_list_clip_and_insert (GList *text_chunk_list,
                                 GList *next)
 {
 #ifdef CLIP_DEBUG
-       if (chunk->string)
-               fprintf (stderr, "clip-and-insert for %s, between %s and %s\n",
+/*     if (chunk->string) */
+               fprintf (stderr, "clip-and-insert for %s, between %s (%d) and %s (%d)\n",
                         chunk->string,
                         (prev && ((TextChunk *)prev->data)->string ? ((TextChunk *)prev->data)->string : "<null>"),
-                        (next && ((TextChunk *)next->data)->string ? ((TextChunk *)next->data)->string : "<null>"));
+                        (prev ? ((TextChunk *)prev->data)->text_bounds.x : -1),
+                        (next && ((TextChunk *)next->data)->string ? ((TextChunk *)next->data)->string : "<null>"),
+                        (next ? ((TextChunk *)next->data)->text_bounds.x : -1));
 #endif
        /* cases: */
        if (!prev && !next) { /* first element in, no clip needed */
-//             if (chunk->string && strlen (chunk->string)) {
-                       text_chunk_list =
-                               g_list_append (text_chunk_list, chunk);
-                       PRINT_CHUNK_DEBUG (chunk, "append",
-                                          prev,
-                                          NULL);
-//             }
+               text_chunk_list =
+                       g_list_append (text_chunk_list, chunk);
+               PRINT_CHUNK_DEBUG (chunk, "append",
+                                  prev,
+                                  NULL);
        } else { /* check for clip with prev */
                /* if we split the prev */
                if (prev &&
@@ -407,10 +411,8 @@ text_chunk_list_clip_and_insert (GList *text_chunk_list,
                                                     (TextChunk *) prev->data)) {
                                text_chunk_tail_clip (prev->data, chunk);
                        }
-//                     if (chunk->string && strlen (chunk->string)) {
-                               text_chunk_list =
-                                       g_list_append (text_chunk_list, chunk);
-//                     }
+                       text_chunk_list =
+                               g_list_append (text_chunk_list, chunk);
                }
        }
        return text_chunk_list;
@@ -468,6 +470,8 @@ review_buffer_get_text_chunk (ScreenReviewBuffer *reviewBuffer,
        role = Accessible_getRole (accessible);
        text_chunk = g_new0 (TextChunk, 1);
        text_chunk->clip_bounds = *bounds;
+       text_chunk->source = accessible;
+       Accessible_ref (accessible);
        if (Accessible_isText (accessible)) {
                text = Accessible_getText (accessible);
                offset = AccessibleText_getOffsetAtPoint (text,
@@ -492,9 +496,10 @@ review_buffer_get_text_chunk (ScreenReviewBuffer *reviewBuffer,
                                 text_chunk->start_char_bounds.x,
                                 text_chunk->start_char_bounds.width);
 #endif
-                       end--; /* XXX: bug workaround */
+                       if (s && strlen (s) && s[strlen (s) - 1] == '\n')
+                               end--;
                        AccessibleText_getCharacterExtents (
-                               text, end--,
+                               text, end - 1,
                                &text_chunk->end_char_bounds.x,
                                &text_chunk->end_char_bounds.y,
                                &text_chunk->end_char_bounds.width,
@@ -502,7 +507,7 @@ review_buffer_get_text_chunk (ScreenReviewBuffer *reviewBuffer,
                                SPI_COORD_TYPE_SCREEN);
 #ifdef CLIP_DEBUG                      
                        fprintf (stderr, "end char (%d) x, width %d %d\n",
-                                end,
+                                end - 1,
                                 text_chunk->end_char_bounds.x,
                                 text_chunk->end_char_bounds.width);
 #endif
@@ -523,33 +528,11 @@ review_buffer_get_text_chunk (ScreenReviewBuffer *reviewBuffer,
                text_chunk->text_bounds.height = y2 - text_chunk->text_bounds.y;
                text_chunk->start_offset = start;
                text_chunk->end_offset = end;
-               if (text_chunk->text_bounds.x < text_chunk->clip_bounds.x) {
-                       text_chunk->text_bounds.x = text_chunk->clip_bounds.x;
-                       text_chunk->text_bounds.isClipped = TRUE;
-               } 
-               if ((text_chunk->text_bounds.x +
-                    text_chunk->text_bounds.width)
-                   > (text_chunk->clip_bounds.x +
-                      text_chunk->clip_bounds.width)) {
-                       text_chunk->text_bounds.width =
-                               MAX (0, (text_chunk->clip_bounds.x +
-                                        text_chunk->clip_bounds.width) -
-                                    text_chunk->text_bounds.x);
-                       text_chunk->text_bounds.isClipped = TRUE;
-               }
-               if (!BOUNDS_CONTAIN_Y (&text_chunk->text_bounds,
-                                      screen_y)) {
-#ifdef CLIP_DEBUG                      
-                       fprintf (stderr, "%s out of bounds (%d-%d)\n", s,
-                                text_chunk->text_bounds.y,
-                                text_chunk->text_bounds.y +
-                                text_chunk->text_bounds.height);
-#endif                 
-                       s = NULL;
-               }
+               AccessibleText_unref (text);
        } else {
                if (role == SPI_ROLE_PUSH_BUTTON ||
                    role == SPI_ROLE_CHECK_BOX ||
+                   role == SPI_ROLE_LABEL ||
                    role == SPI_ROLE_MENU ||
                    role == SPI_ROLE_MENU_ITEM) { /* don't like this
                                                     special casing :-( */
@@ -560,12 +543,36 @@ review_buffer_get_text_chunk (ScreenReviewBuffer *reviewBuffer,
                        text_chunk->end_offset = strlen (s);
                }
        }
+       if (text_chunk->text_bounds.x < text_chunk->clip_bounds.x) {
+               text_chunk->text_bounds.x = text_chunk->clip_bounds.x;
+               text_chunk->text_bounds.isClipped = TRUE;
+       } 
+       if ((text_chunk->text_bounds.x +
+            text_chunk->text_bounds.width)
+           > (text_chunk->clip_bounds.x +
+              text_chunk->clip_bounds.width)) {
+               text_chunk->text_bounds.width =
+                       MAX (0, (text_chunk->clip_bounds.x +
+                                text_chunk->clip_bounds.width) -
+                            text_chunk->text_bounds.x);
+               text_chunk->text_bounds.isClipped = TRUE;
+       }
+       if (!BOUNDS_CONTAIN_Y (&text_chunk->text_bounds,
+                              screen_y)) {
+#ifdef CLIP_DEBUG                      
+               fprintf (stderr, "%s out of bounds (%d-%d)\n", s,
+                        text_chunk->text_bounds.y,
+                        text_chunk->text_bounds.y +
+                        text_chunk->text_bounds.height);
+#endif                 
+               s = NULL;
+               text_chunk->start_offset = offset;
+               text_chunk->end_offset = offset;
+       }
        if (s && strlen (s)) {
                if (s[strlen(s)-1] == '\n') s[strlen(s)-1] = ' ';
                /* XXX: if last char is newline, aren't its bounds wrong now? */
                text_chunk->string = s;
-               text_chunk->source = text;
-               if (text) AccessibleText_ref (text);
 #ifdef CLIP_DEBUG
                fprintf (stderr, "%s, bounds %d-%d; clip %d-%d\n",
                         s,
@@ -576,9 +583,7 @@ review_buffer_get_text_chunk (ScreenReviewBuffer *reviewBuffer,
 #endif         
        } else {
                text_chunk->string = NULL;
-               text_chunk->source = NULL;
        }
-       if (text) AccessibleText_unref (text);
        return text_chunk;
 }
 
@@ -660,68 +665,243 @@ clip_into_buffers (Accessible *accessible,  BoundaryRect* parentClipBounds[],
 #undef CHARACTER_CLIP_DEBUG
 
 static char*
-text_chunk_get_clipped_string (TextChunk *chunk)
+text_chunk_get_clipped_substring_by_char (TextChunk *chunk, int start, int end)
 {
-       char *s;
+       BoundaryRect char_bounds;
        int i;
-       int len;
-       gunichar c;
+       char *s;
        GString *string = g_string_new ("");
-       BoundaryRect char_bounds;
-       if (!chunk->text_bounds.isClipped)
-               s = chunk->string;
-       else if (chunk->source) {
-               len = chunk->end_offset - chunk->start_offset;
-#ifdef CHARACTER_CLIP_DEBUG            
-               fprintf (stderr, "clipping %s\n", chunk->string);
-#endif         
-               for (i = chunk->start_offset; i < chunk->end_offset; ++i) {
-                       AccessibleText_getCharacterExtents (chunk->source,
-                                                           i,
-                                                           &char_bounds.x,
-                                                           &char_bounds.y,
-                                                           &char_bounds.width,
-                                                           &char_bounds.height,
-                                                           SPI_COORD_TYPE_SCREEN);
+       gunichar c;
+       AccessibleText *text = Accessible_getText (chunk->source);
+       for (i = start; i < end; ++i) {
+               AccessibleText_getCharacterExtents (text,
+                                                   i,
+                                                   &char_bounds.x,
+                                                   &char_bounds.y,
+                                                   &char_bounds.width,
+                                                   &char_bounds.height,
+                                                   SPI_COORD_TYPE_SCREEN);
 #ifdef CHARACTER_CLIP_DEBUG
-                       fprintf (stderr, "testing %d-%d against %d-%d\n",
-                                char_bounds.x, char_bounds.x+char_bounds.width,
-                                chunk->text_bounds.x,
-                                chunk->text_bounds.x + chunk->text_bounds.width);
+               fprintf (stderr, "testing %d-%d against %d-%d\n",
+                        char_bounds.x, char_bounds.x+char_bounds.width,
+                        chunk->text_bounds.x,
+                        chunk->text_bounds.x + chunk->text_bounds.width);
 #endif
-                       if (BOUNDS_CONTAIN_X (&chunk->text_bounds,
-                                             char_bounds.x)) {
-                               c = AccessibleText_getCharacterAtOffset (
-                                       chunk->source, i);
+               if (BOUNDS_CONTAIN_X_BOUNDS (chunk->text_bounds,
+                                            char_bounds)) {
+                       c = AccessibleText_getCharacterAtOffset (
+                               text, i);
 #ifdef CLIP_DEBUG                              
-                               fprintf (stderr, "[%c]", c);
+                       fprintf (stderr, "[%c]", c);
 #endif                         
-                               g_string_append_unichar (string, c);
-                       }
+                       g_string_append_unichar (string, c);
                }
-               s = string->str;
-       } else { /* we're clipped, but don't implement AccessibleText :-( */
-               /* punt for now, maybe we can do betterc someday */
-               s = chunk->string;
        }
+       AccessibleText_unref (text);
+       s = string->str;
        g_string_free (string, FALSE);
        return s;
 }
 
+
+/*
+ * Note: this routine shouldn't have to do as much as it currently does,
+ *       but at the moment it works around another bug (probably one in this
+ *       code).
+ */
+static char *
+string_strip_newlines (char *s, long offset, long *start_offset, long *end_offset)
+{
+       int i;
+       char *word_start = s;
+       /* FIXME: potential memory leak here */
+       for (i=0; s && s[i]; ++i)
+       {
+               if (s [i] == '\n' && i > (offset - *start_offset) ) {
+                       s [i] = '\0';
+                       *end_offset = *start_offset + i;
+                       return word_start;
+               } else if (s [i] == '\n') {
+                       word_start = &s[i + 1];
+               }
+       }
+       return word_start;
+}
+
+static char *
+string_guess_clip (TextChunk *chunk)
+{
+       BoundaryRect b;
+       char *s = NULL, *sp = chunk->string, *ep;
+       long start_offset, end_offset, len;
+       if (sp) {
+               AccessibleComponent *component =
+                       Accessible_getComponent (chunk->source);
+               ep = sp + (strlen (sp));
+               len = g_utf8_strlen (chunk->string, -1);
+               if (component) {
+                       AccessibleComponent_getExtents (component,
+                                                       &b.x, &b.y,
+                                                       &b.width, &b.height,
+                                                       SPI_COORD_TYPE_SCREEN);
+                       start_offset = len * (chunk->text_bounds.x - b.x) / b.width;
+                       end_offset = len * (chunk->text_bounds.x +
+                                           chunk->text_bounds.width - b.x) / b.width;
+                       fprintf (stderr, "String len %d, clipped to %d-%d\n",
+                                len, start_offset, end_offset);
+                       len = end_offset - start_offset;
+                       sp = g_utf8_offset_to_pointer (chunk->string, start_offset);
+                       ep = g_utf8_offset_to_pointer (chunk->string, end_offset);
+               }
+               s = g_new0 (char, ep - sp + 1);
+               s = g_utf8_strncpy (s, sp, len);
+               s [sp - ep] = '\0';
+               g_assert (g_utf8_validate (s, -1, NULL));
+       }
+       return s;
+}
+
+static char*
+text_chunk_get_clipped_string (TextChunk *chunk)
+{
+       char *s, *string = "";
+       int i;
+       long start = chunk->start_offset, end = chunk->end_offset;
+       long word_start, word_end, range_end;
+       BoundaryRect start_bounds, end_bounds;
+       gboolean start_inside, end_inside;
+       if (!chunk->text_bounds.isClipped || !chunk->string)
+               string = chunk->string;
+       else if (chunk->source && Accessible_isText (chunk->source)) {
+               /* while words at offset lie within the bounds, add them */
+               AccessibleText *text = Accessible_getText (chunk->source);
+#ifdef CLIP_DEBUG              
+               fprintf (stderr, "clipping %s\n", chunk->string);
+#endif
+               do {
+                   s = AccessibleText_getTextAtOffset (text,
+                                                       start,
+                                               SPI_TEXT_BOUNDARY_WORD_END,
+                                                       &word_start,
+                                                       &word_end);
+                   range_end = word_end;
+                   s = string_strip_newlines (s, start, &word_start, &word_end);
+                   AccessibleText_getCharacterExtents (text,
+                                                       word_start,
+                                                       &start_bounds.x,
+                                                       &start_bounds.y,
+                                                       &start_bounds.width,
+                                                       &start_bounds.height,
+                                                       SPI_COORD_TYPE_SCREEN);
+                   AccessibleText_getCharacterExtents (text,
+                                                       word_end - 1,
+                                                       &end_bounds.x,
+                                                       &end_bounds.y,
+                                                       &end_bounds.width,
+                                                       &end_bounds.height,
+                                                       SPI_COORD_TYPE_SCREEN);
+                   start_inside = BOUNDS_CONTAIN_X_BOUNDS (chunk->text_bounds,
+                                                           start_bounds);
+                   end_inside = BOUNDS_CONTAIN_X_BOUNDS (chunk->text_bounds,
+                                                         end_bounds);
+                   if (start_inside && end_inside) {
+                           /* word is contained in bounds */
+                           string = g_strconcat (string, s, NULL);
+                   } else if (start_inside || end_inside) {
+                           /* one end of word is in */
+                           if (word_end > end) word_end = end;
+                           s = text_chunk_get_clipped_substring_by_char (
+                                   chunk,
+                                   MAX (word_start, chunk->start_offset),
+                                   MIN (word_end, chunk->end_offset));
+                           string = g_strconcat (string, s, NULL);
+                   } else {
+                   }
+                   start = range_end;
+               } while (start < chunk->end_offset);
+       } else { /* we're clipped, but don't implement AccessibleText :-( */
+               /* guess for now, maybe we can do better someday */
+               string = string_guess_clip (chunk);
+       }
+       return string;
+}
+
+
+static char*
+text_chunk_pad_string (TextChunk *chunk, char *string, glong offset, const char *pad_chars)
+{
+       char *s = "";
+       char *cp;
+       char startbuf[6], padbuf[6], endbuf[6];
+       int pixels_per_column = 6;
+        /* this is an arbitrary pixel-to-textcolumn mapping at present */
+       glong end_padding;
+       gint howmany;
+       howmany = g_unichar_to_utf8 (g_utf8_get_char (pad_chars), startbuf);
+       startbuf[howmany] = '\0';
+       g_assert (howmany < 7 && howmany > 0);
+       cp = g_utf8_find_next_char (pad_chars, NULL);
+       howmany = g_unichar_to_utf8 (g_utf8_get_char (cp), padbuf);
+       padbuf[howmany] = '\0';
+       g_assert (howmany < 7 && howmany > 0);
+       cp = g_utf8_find_next_char (cp, NULL);
+       howmany = g_unichar_to_utf8 (g_utf8_get_char (cp), endbuf);
+       endbuf[howmany] = '\0';
+       g_assert (howmany < 7 && howmany > 0);
+       end_padding = chunk->clip_bounds.x / pixels_per_column; 
+       while (offset < end_padding - 1) {
+               s = g_strconcat (s, padbuf, NULL); /* could be more efficient */
+               ++offset;
+       }
+       s = g_strconcat (s, startbuf, string, NULL);
+       offset += g_utf8_strlen (string, -1) + 1;
+       end_padding = chunk->text_bounds.x / pixels_per_column; 
+       while (offset < end_padding) {
+               s = g_strconcat (s, padbuf, NULL); /* could be more efficient */
+               ++offset;
+       }
+       end_padding = (chunk->clip_bounds.x + chunk->clip_bounds.width) /
+               pixels_per_column;
+       while (offset < end_padding - 1) {
+               s = g_strconcat (s, padbuf, NULL); /* could be more efficient */
+               ++offset;
+       }
+       s = g_strconcat (s, endbuf, NULL);
+       return s;
+}
+
+static char*
+text_chunk_to_string (TextChunk *chunk, glong offset)
+{
+       char *s = NULL;
+       if (chunk->string) {
+               s = text_chunk_get_clipped_string (chunk);
+               if (chunk->clip_bounds.role == SPI_ROLE_PUSH_BUTTON) {
+                       s = text_chunk_pad_string (chunk, s, offset, "[ ]");
+               } else if (chunk->clip_bounds.role == SPI_ROLE_FRAME) {
+                       s = text_chunk_pad_string (chunk, s, offset, "| |");
+               } else if (chunk->clip_bounds.role == SPI_ROLE_TEXT) {
+                       s = text_chunk_pad_string (chunk, s, offset, "\" \"");
+               } else {
+                       s = text_chunk_pad_string (chunk, s, offset, "   ");
+               }
+       }
+       return s;
+}
+
 static char*
 text_chunk_list_to_string (GList *iter)
 {
        char *s = "";
        char *string;
        TextChunk *chunk = NULL;
+       int offset = 0;
        while (iter) {
                chunk = (TextChunk *)iter->data;
-               if (chunk /* && chunk->string */) {
-                       string = text_chunk_get_clipped_string (chunk);
+               if (chunk) {
+                       string = text_chunk_to_string (chunk, g_utf8_strlen (s, -1));
                        if (string)
-                               s = g_strconcat (s, "|", string, NULL);
-                       else /* XXX test */
-                               s = g_strconcat (s, ":", NULL);
+                               s = g_strconcat (s, string, NULL);
                }
                iter = iter->next;
        }
@@ -729,22 +909,66 @@ text_chunk_list_to_string (GList *iter)
        return s;
 }
 
+#define COMPOSITE_DEBUG
+
+static void
+toplevel_composite (ScreenReviewBuffer *buffers[])
+{
+       int i;
+       GList *chunk_list, *iter;
+       TextChunk *chunk;
+
+       chunk_list = buffers[SPI_LAYER_CANVAS]->text_chunks;
+       for (i = SPI_LAYER_MDI; i < SPI_LAYER_OVERLAY; ++i) {
+               iter = buffers[i]->text_chunks;
+#ifdef COMPOSITE_DEBUG
+               fprintf (stderr, "layer %d has %d chunks\n",
+                        i, g_list_length (iter));
+#endif         
+               while (iter) {
+                       chunk = (TextChunk *) iter->data;
+                       if (chunk) {
+#ifdef COMPOSITE_DEBUG
+                               fprintf (stderr, "inserting chunk <%s>\n",
+                                        chunk->string ? chunk->string : "<null>");
+#endif
+                               chunk_list =
+                                       text_chunk_list_insert_chunk (chunk_list,
+                                                                     chunk);
+                       }
+                       iter = iter->next;
+               }
+       }
+}
+
 static char*
 review_buffer_composite (ScreenReviewBuffer *buffers[])
 {
+       /* TODO: FIXME: something is wrong here, compositing fails */
        int i;
        GList *chunk_list, *iter;
        TextChunk *chunk;
-       chunk_list = buffers[0]->text_chunks;
-/*     for (i = 1; i < SPI_LAYER_LAST_DEFINED; ++i) {
+       chunk_list = buffers[SPI_LAYER_BACKGROUND]->text_chunks;
+       for (i = 2; i < SPI_LAYER_LAST_DEFINED; ++i) {
+               if (i == SPI_LAYER_WIDGET) i = SPI_LAYER_OVERLAY;
+               /*
+                * Q: why skip these layers ?
+                * A: since layers WIDGET, MDI, and POPUP have already been
+                *  composited into layer CANVAS for each toplevel before this
+                *  routine is called.
+                */
                iter = buffers[i]->text_chunks;
+#ifdef CLIP_DEBUG
                fprintf (stderr, "layer %d has %d chunks\n",
                         i, g_list_length (iter));
+#endif         
                while (iter) {
                        chunk = (TextChunk *) iter->data;
                        if (chunk) {
+#ifdef CLIP_DEBUG
                                fprintf (stderr, "inserting chunk <%s>\n",
                                         chunk->string ? chunk->string : "<null>");
+#endif
                                chunk_list =
                                        text_chunk_list_insert_chunk (chunk_list,
                                                                      chunk);
@@ -752,7 +976,8 @@ review_buffer_composite (ScreenReviewBuffer *buffers[])
                        iter = iter->next;
                }
        }
-*/     chunk_list = buffers[SPI_LAYER_WIDGET]->text_chunks;
+       
+       chunk_list = buffers[SPI_LAYER_WIDGET]->text_chunks;
        return text_chunk_list_to_string (chunk_list);
 }
 
@@ -771,7 +996,7 @@ get_screen_review_line_at (int x, int y)
   GTimer *timer = g_timer_new ();
   int i;
 
-  for (i = 0; i < SPI_LAYER_LAST_DEFINED; ++i) {
+  for (i = 1; i < SPI_LAYER_LAST_DEFINED; ++i) {
          reviewBuffers[i] = g_new0 (ScreenReviewBuffer, 1);
          clip_bounds[i] = g_new0 (BoundaryRect, 1);
          clip_bounds[i]->isClipped = FALSE;
@@ -833,12 +1058,14 @@ get_screen_review_line_at (int x, int y)
                                                      &toplevel_bounds.height,
                                                      SPI_COORD_TYPE_SCREEN);
                      toplevel_bounds.isEmpty = FALSE;
-                     for (i = 0; i < SPI_LAYER_LAST_DEFINED; ++i) {
+                     for (i = 1; i < SPI_LAYER_LAST_DEFINED; ++i) {
                              *clip_bounds[i] = toplevel_bounds;
                      }
                      clip_into_buffers (toplevel, clip_bounds,
                                     reviewBuffers, x, y);
-#ifdef CLIP_DEBUG
+
+                     toplevel_composite (reviewBuffers);
+#ifdef CHUNK_LIST_DEBUG
                      fprintf (stderr, "toplevel clip done\n");
                      debug_chunk_list (reviewBuffers[SPI_LAYER_WIDGET]->text_chunks);
 #endif