Re: Bug?

Charlie Reitzel <[email protected]> Wed, 25 Jan 2006 09:24:45 -0500
Newsgroups gmane.comp.web.html-tidy.devel
Message-ID <[email protected]>
Good eye, Cory.  Wrt to the lexer.c issue, while it appears to be 
intentional, it also appears to be wrong!  That attrs.c line, imo, is just 
wrong.

That said, it is not immediately clear what is right.  Time for a test case 
or 3 to exercise these bits of code.

1) Lexer, see http://tidy.sf.net/issue/427846

I would expand the scope of inquiry just a bit. Bitwise operations are not 
appropriate on token modes: IgnoreWhitespace, MixedContent, Preformatted, 
IgnoreMarkup and CdataContent.  As non-negative integers (0, 1, 2, 3, ...) 
they are appropriate only for equality tests.  I do not think relational 
ops (<, <=, >=, >) have any meaning on these values.

The only place I see that happening is on line 1975.  This is evaluated 
only if the the previous trailing blank was trimmed in TrimTrailingSpace() 
in parser.c.   I would propose fixing the test and re-testing bug 427846 to 
see if the markup is what we want.  If not, I propose dropping the 
Lexer.insertspace state variable altogether.

A quick scan on the other modes:

MixedContent - never explicitly tested, set frequently.
Preformatted - several bitwise ops in parser.c!
IgnoreMarkup - all tests correct
CdataContent - all tests correct


2) attrs.c - Content Model

Restate as nodeHasCM(node, CM_TABLE|CM_ROW)

This change is relatively recent (v1.110 of attrs.c), so there is probably 
a test case for this change already if I know Arnaud.

Note: perhaps this tiny function should be refactored as a macro.  Ditto 
for the whole content model nodeXXXCM() family of functions.

take it easy,
Charlie

At 03:45 AM 1/25/2006 -0800, Cory Nelson wrote:
>On 1/25/06, Bjoern Hoehrmann <[email protected]> wrote:
> > * Cory Nelson wrote:
> > >IgnoreWhitespace is defined to 0, and in lexer.c line 1989 there is a
> > >bitwise being done with it (mode & IgnoreWhitespace).  Bug?
> >
> > http://cvs.sourceforge.net/viewcvs.py/tidy/tidy/src/lexer.c?r1=1.3&r2=1.4
> > Maybe Charlie knows? Major changes to whitespace processing have been
> > made since, so this might just do what we want.
>
>It will always evaluate to 0, so maybe it should be taken out if that
>was intentional.
>
>I also noticed attrs.c line 1370 - there is an expression
>(node->tag->model & CM_TABLE|CM_ROW).  Bitwise AND takes precedence
>over bitwise OR and the spacing suggests the author really wanted
>(node->tag->model & (CM_TABLE|CM_ROW)).
>
> > Björn Höhrmann
>
>--
>Cory Nelson



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid3432&bid#0486&dat1642