Questions about access.c

[aditsu ()]

[resending after subscribing]

Hi, I was looking through the code, and I found a couple of things that I didn't quite understand:
- why does GetRgb use strstr with color names?
- when checking for d-links in CheckImage (lines 821 and 852) why does it use strcmp? doesn't seem to match the comments
- also in CheckImage, what's up with the empty ifs at line 861?
- getTextNode doesn't seem to do anything for non-text nodes, shouldn't the content traversal be in an "else" branch?

Could somebody take a look?

Thanks
Adrian

[Arnaud Desitter-2]

Hi,

access.c was not part of tidy originally and was originally written
seperately. You probably noticed that its quality is not as good as
other parts of tidy.

On 03/07/2009, aditsu <aditsu@...> wrote:
> [resending after subscribing]
>
> Hi, I was looking through the code, and I found a couple of things that I
> didn't quite understand:
> - why does GetRgb use strstr with color names?

It looks wrong. strcmp should be used.

> - when checking for d-links in CheckImage (lines 821 and 852) why does it
> use strcmp? doesn't seem to match the comments

It looks wrong as well.

> - also in CheckImage, what's up with the empty ifs at line 861?

Not sure. Check the history in CVS.

> - getTextNode doesn't seem to do anything for non-text nodes, shouldn't the
> content traversal be in an "else" branch?

The code looks pretty bad as well.

There is no real point in fixing the code it if some test cases are
not added to the test suite to illustrate why the code change is
useful.

There are a bunch of patches in the tracker waiting to be merged. I
have very little time for tidy these days unfortunately.

Regards,

>
> Could somebody take a look?
>
> Thanks
> Adrian
> --
> View this message in context: http://www.nabble.com/Questions-about-access.c-tp24313470p24313470.html
> Sent from the w3.org - html-tidy mailing list archive at Nabble.com.
>
>
>
>

[aditsu]

Hi, thanks for your answer

Arnaud Desitter-2 wrote:

access.c was not part of tidy originally and was originally written seperately.

Oh, I wasn't aware of that. It happened to be the first source file in alphabetical order (in the src folder).

There is no real point in fixing the code it if some test cases are
not added to the test suite to illustrate why the code change is
useful.

You have a point there. Perhaps the author(s) of that code should be involved.
My main focus is updating jtidy (and translating access.c to java was going to be part of that), I don't expect to also write patches for tidy (at least not before I'm happy with jtidy).
Since the accessibility code is kinda separate (and can actually be excluded from compilation), maybe I should focus on other things first.

Thanks
Adrian

[Arnaud Desitter-2]

Hi,

You are welcome to report bugs with test cases and to provide patches
in you change your mind though.
"tidy/test" contains the test suite which may be useful to you.

Good luck,

On 03/07/2009, aditsu <aditsu@...> wrote:

>
> Hi, thanks for your answer
>
> Arnaud Desitter-2 wrote:
> >
> > access.c was not part of tidy originally and was originally written
> > seperately.
> >
>
> Oh, I wasn't aware of that. It happened to be the first source file in
> alphabetical order (in the src folder).
>
>
> > There is no real point in fixing the code it if some test cases are
> > not added to the test suite to illustrate why the code change is
> > useful.
> >
>
> You have a point there. Perhaps the author(s) of that code should be
> involved.
> My main focus is updating jtidy (and translating access.c to java was going
> to be part of that), I don't expect to also write patches for tidy (at least
> not before I'm happy with jtidy).
> Since the accessibility code is kinda separate (and can actually be excluded
> from compilation), maybe I should focus on other things first.
>
> Thanks
> Adrian
> --
> View this message in context: http://www.nabble.com/Questions-about-access.c-tp24313470p24326229.html
> Sent from the w3.org - html-tidy mailing list archive at Nabble.com.
>
>
>