Re: lynx-dev When unhighlighting some highlighted links, lynx draws them incorrectly

[Klaus Weide]

On Sat, 13 Mar 1999, Vlad Harchev wrote:
> On Fri, 5 Mar 1999, Vlad Harchev wrote:
> 
> > 
> >   When unhighlighting highlighted links that contain some extra formatting,
> >   lynx-2.8.2dev12 and lynx-2.8 draw them incorrectly ( compared to 
> >   the way the were  drawn before becoming highlighted first time)  when 
> >   compiled with lss support [...]

>   Here is a patch that will fix this unhighlighting bug in lynx-dev19
> compiled with lss support. Original behaviour can be restored by defining
> NO_HILIT_FIX.

> diff -ruP 2.8.2dev19-orig/src/GridText.c lynx-2.8.2dev19-fixed/src/GridText.c
> --- lynx-2.8.2dev19-orig/src/GridText.c       Tue Mar  9 22:45:03 1999
> +++ lynx-2.8.2dev19-fixed/src/GridText.c      Sat Mar 13 05:17:34 1999
> @@ -10278,3 +10278,234 @@
> +/*
> + this function draws the part of line 'line', pointed by 'str' (which can be 
> + non terminated with null - ie be line->data+N ) drawing 
> + 'len' bytes (not characters) of it. 

I thought data always *is* terminated with null - is that not true?

> + It doesn't check whether the 'len' 
> + bytes cross a character boundary (if multibyte chars are in string). 
> + Assumes that the cursor is positioned in the place where the 1st char of 
> + string should be drawn. [...]
> + This code is based on display_line. [...]
> +*/
> +PRIVATE void redraw_part_of_line ARGS4( 
> +        HTLine *,    line,
> +        char*,          str,
> +        int,            len,
> +     HText *,        text)
> +{
...........
> +    /* this assumes that the part of line to be drawn fits in the screen*/
> +    while (  data < end_of_data ) {
> +        buffer[0] = *data;
> +     data++;
> +
> +#if defined(USE_COLOR_STYLE) || defined(SLSC)
> +#define CStyle line->styles[current_style]
> +
> +     while (current_style < line->numstyles &&
> +            i >= (int) (CStyle.horizpos + line->offset + 1))
> +     {
> +             LynxChangeStyle (CStyle.style,CStyle.direction,CStyle.previous);
> +             current_style++;
> +     }
> +#endif

I suspect that this can create the kind of problems my patch is
trying to resolve - LynxChangeStyle uses a "stack" of styles, which
can get messed up if it's used for non-contiguous sections of text
where style changes are not properly nested...  I haven't tested
it though.  We can try to use your method and see whether it creates
problems.

(These would be the kinds of problems you don't see if your lss file
is simple or if you only look at "simple" documents; they may ony
occur when the anchor is on more than one line, or with some more
unusual conditions added...)

Also, on the first iteration  LynxChangeStyle is called for all style
changes on the current line that should come *before* the current
position.  That doesn't look like a good thing, although maybe it
happens to have no visible bad effect.

> +     switch (buffer[0]) {
> +
...........
> +     } /* end of switch */
> +    } /* end of while */
> +
> +#ifndef USE_COLOR_STYLE
> +    stop_underline();
> +    stop_bold();
> +#else
> +    while (current_style < line->numstyles)
> +    {
> +     LynxChangeStyle (CStyle.style, CStyle.direction, CStyle.previous);
> +     current_style++;
> +    }

Here again, doing LynxChangeStyle for all style changes *after* the anchor
may not be a good thing.

> +#undef CStyle
> +#endif
> +    return;
> +}
......

> +PUBLIC void redraw_lines_of_link ARGS1( 
> +            int , cur )
> +{
> +#if defined(USE_COLOR_STYLE) && !defined(NO_HILIT_FIX)        
> +#define pvtTITLE_HEIGHT 1
> +    HTLine* todr1,*todr2;
> +    int lines_back;                    
> +    if (HTMainText->next_line == HTMainText->last_line) {

Somehow I never realized that there was a HTMainText->next_line
that can be used for stuff like this...

I'm not sure it will always be set correctly; there may be a border
case you missed when (HTMainText->next_line == HTMainText->last_line)
is true but 

> +    /* we are at the last page - that is partially filled */

is not true.

> +       lines_back = HTMainText->Lines - ( links[cur].ly-pvtTITLE_HEIGHT+
> +       HTMainText->top_of_screen);
> +    } else  {               
> +       lines_back=display_lines - (links[cur].ly-pvtTITLE_HEIGHT);
> +    };
> +    todr1=HTMainText->next_line;
> +    while (lines_back--) todr1=todr1->prev;

A cleaner / clearer way for this (finding next or last line, then going
backwards) would be to use HTMainText->top_of_screen_line.  It seems
to be meant as a pointer to the first line on the screen (see comments
in GridText.c before 'struct _HText {'), but is currently never used
or set; but setting it the right way in display_page should be a
trivial change.

By the way, in the GridText.c comment:

    /*      Notes on struct _Htext:
    **      next_line is valid if state is false.

I suspect that "state" is an ancient typo for "stale".


    Klaus