Wednesday, 5 March 2014

Re: [PATCH] implementation of hr tag in libdom

I apologise for how long this has taken to do, I have been very tired :-(

Firstly, well done on getting git send-email to work :-)

> diff --git a/include/dom/dom.h b/include/dom/dom.h
> index 2ef6e9b..d34f27f 100644
> --- a/include/dom/dom.h
> +++ b/include/dom/dom.h
> @@ -55,6 +55,7 @@
> #include <dom/html/html_option_element.h>
> #include <dom/html/html_select_element.h>
> #include <dom/html/html_options_collection.h>
> +#include <dom/html/html_hr_element.h>

You've not moved this anywhere and also not told me why you put it in this
position.

> diff --git a/include/dom/html/html_hr_element.h b/include/dom/html/html_hr_element.h
> * Copyright 2009 Bo Yang <struggleyb.nku@gmail.com>
> + * Copyright 2014 Rupinder Singh Khokhar <rsk1coder99@gmail.com>

Much better :-)

> +dom_exception dom_html_h_r_element_get_no_shade(
> + dom_html_h_r_element *ele, bool *no_shade);

The no_shade is good, but I'm slightly confused as to why you're putting
dom_html_h_r_element rather than dom_html_hr_element -- does our conversion
algorithm really expect it this way?

[snip rest]

If the test suite really does expect dom_html_h_r_element then I guess I can
accept this patch for now, but overall, I think I'd prefer it if it could be
dom_html_hr_element -- Perhaps John Mark will have some useful input here?

D.

--
Daniel Silverstone http://www.netsurf-browser.org/
PGP mail accepted and encouraged. Key Id: 3CCE BABE 206C 3B69

No comments:

Post a Comment