Re: lynx-dev dev.16 patch 3 - HTMIME.c, HTTP.c redirection

[Leonid Pauzner]

5-Dec-99 06:14 Klaus Weide wrote:
> On Sun, 5 Dec 1999, Leonid Pauzner wrote:

>> 4-Dec-99 09:10 Klaus Weide wrote:
>>
>> > * Save size for regular files in anchor structure (we did the stat() 
>> > anyway),
>> >   it shows up on INFO page as Content-Length.  Also use it in partial 
>> > display
>> >   mode to prevent the last call to HTDisplayPartial from HTFileCopy, since 
>> > a
>> >   call to HText_pageDisplay will follow immediately.  (But note that 
>> > nothing
>> >   important depends on the correct size; should it be wrong, we lose at 
>> > most
>> >   one partial display screen update.  An equivalent suppression of the last
>> >   partial update for network protocols is not recommended, since (a) the 
>> > size
>> >   is more unreliable and (b) the socket FIN may get delayed by the 
>> > network.)
>>
>>
>> > Index: 2.30/WWW/Library/Implementation/HTFormat.c
>> > --- 2.30/WWW/Library/Implementation/HTFormat.c Sat, 04 Dec 1999 00:06:06 
>> > -0600
>> > +++ 2.30(w)/WWW/Library/Implementation/HTFormat.c Sat, 04 Dec 1999 
>> > 01:59:43 -0600
>> > @@ -795,7 +807,9 @@
>> >       (*targetClass.put_block)(sink, input_buffer, status);
>> >       bytes += status;
>> >       HTReadProgress(bytes, 0);
>> > -     HTDisplayPartial();
>> > +     if (NumOfLines_partial >= 0 && HTMainText && HTMainAnchor &&
>> > +         bytes != HTMainAnchor->content_length)
>> > +         HTDisplayPartial();
>>
>> >       if (HTCheckForInterrupt()) {
>> >           _HTProgress (TRANSFER_INTERRUPTED);
>>
>> That looks excessive, at least very strange as of dev16 changes
>> which enable partial mode right in HText_new() so first three
>> conditions are not required and the forth looks rather potetical.

> YM poetical? :)

> I admit I haven't kept up with all the recent patches for partial mode.
> Well I briefly scanned tha patches, they looked like very worthwhile
> improvements, but I haven't tried to really understand them.  I'll wait
> for dev.17 for that, I think.
No, I mean pre-dev16 changes that are already in dev16 at www.slcc.edu
(my post-dev16 changes are irrelevant in this case).

> The tests may be excessive or even wrong, but were the best I could
> think of without changing HTFileCopy to take an anchor as an additional
> parameter (maybe that should be done instead).  Relying on the global
> HTMainText/HTMainAnchor isn't nice, but seemed no worse than what the rest
> of display_partial code is doing - i.e. the assumption that HTMainText
> describes the right document (the one we are *currently* loading for).
dev16 can say this for sure: we enable 'display_partial' when
HText_new() already started, and disable 'display_partial' right after
HTDisplayPartial() in HTFinishDisplayPartial() called few lines below
from the same function. No untested intermediate levels.

> Actually, the tests in the if() are there to make sure HTMainAnchor
> is valid (not NULL), just to be on the safe side.  Testing for both
> becasue I wasn't sure HTMainAnchor always gets reset when HTMainText
> gets reset.  It may all be overkill.
HTMainText and HTMainAnchor are always reset in HText_new(),
they may be wrong before HTDisplayPartial() call unless your check for
'display_partial' (or, previously, NumOfLines_partial >= 0) explicitely.

>> Also, display_page() were hacked by you to redisplay only those lines
>> which were not displayed in partial mode yet so the next repaint from
>> mainloop will not make any harm.

> That doesn't prevent all redisplaying.  In particular I was thinking
HTDisplayPartial() does. You assume the complete file will fit into one
screen and assume very strict timing conditions when your fix can be in
effect.
In the case of TRST mode you may want partial redisplay in more
situation, not less. E.g. call HText_padeDisplay() when TRST table
started on (or above) the current screen but finished below the screen.
But this will add a next level of complexity... add HTDisplayPartial()
call in HTML.c or (better) TRSTable.c when TRST mode is closed.

> of the TRST 'flicker'.  Also at least the title line always gets
> rewritten.  Also some links get repainted anyway (form fields only?),
> in UTF-8 and/or CJK mode things may get repainted anyway, maybe there
> are some other conditions.  I didn't check all the details for this...
> Avoiding unnecessary screen repainting is A Good Thing if Lynx is run
> remotely, although you won't notice the difference if Lynx is running
> on a local machine.

I use Lynx remotely every day, I have not seen a problem here, perhaps
display optimized by ncurses (obsolete 1.9.9e one). [And I have not
using utf-8 display in any way. Also, lynx utf-8 display staff
completely out of my understanding, say, few lines right on the top of
HText_new()].

> Also, it isn't very logical to do a "partial" screen update when we
> we already *know* with reasonable certainty that the file is complete.

> So I would like to keep the suppression of the unnecessary screen
> update, but invite you to suggest a better test..

>> Instead, that may be a good idea to add a true content_length to
>> HTReadProgress() call which may be useful for large files.

I mean in HTReadProgress() in HTFileCopy() from the patch cited above :)

> HTReadProgress() is simply counting wrong (that isn't new).  The
> functions in HTFormat.c don't know anything about the boundary between
> HTTP header and body.  But content_length (for HTTP) counts only body
> bytes.  Proper counting for HTTP resources would have to zero the
> counter at that boundary, the only good place to do that would be in
> HTMIME.c.  But you may still want to take the initial packet(s) into
> account for the bytes/sec, and then it gets more complicated.

Well, I grep WWW for HTReadProgress() just now. There are many calls for
bytes = 0 but all calls with bytes > 0 are in a proper places only. So
we can initialize bytes/sec counter with the first call for (bytes > 0),
not (bytes == 0) in HTReadProgress() implementation in HTAlert.c. That's
easy fix.


>> Of cause, that should not be HTMainAnchor reference but
>> a reference to anchor from some other place.

> Yes, it cannot be a reference to HTMainAnchor for HTReadProgress().
> After all we may be 'd'ownloading, and it may not even be HTML but
> something passed to an external VIEWER.


>    Klaus