[Webkit-unassigned] [Bug 33944] [Android] bindings/v8/V8DOMWrapper.h[cpp] are missing guards for SVG, XPATH and XSLT features

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33944


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #47110|review?                     |review+
               Flag|                            |




--- Comment #2 from David Levin <levin at chromium.org>  2010-01-21 09:14:15 PST ---
(From update of attachment 47110)
Just a few minor things to address.

> Index: WebCore/ChangeLog
> +2010-01-21  Andrei Popescu  <andreip at google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [Android] bindings/v8/V8DOMWrapper.h[cpp] are missing guards for SVG, XPATH and XSLT features
> +        https://bugs.webkit.org/show_bug.cgi?id=33944
> +
> +        Iniside V8DOMWrapper.h[cpp], the code for SVG, XPATH and XSLT features is not guarded

typo: Iniside

Please adjust the comment appropriately when you remove the if ENABLE(SVG) for
the SVG headers (see comment below).

> Index: WebCore/bindings/v8/V8DOMWrapper.cpp
>  #include "Notification.h"
> -#include "SVGElementInstance.h"

Please leave this here and add #include "SVGPathSeg.h" (see comments below).

>  #include "ScriptController.h"
>  #include "V8AbstractEventListener.h"
>  #include "V8Binding.h"
> @@ -63,6 +61,7 @@
>  #include "WorkerContextExecutionProxy.h"
>  
>  #if ENABLE(SVG)
> +#include "SVGElementInstance.h"

SVGElementInstance.h correctly has #if ENABLE(SVG) inside of it so this is
unnecessary. (If something is wrong with the way the header does the if ENABLE,
please fix that.)

>  #include "SVGPathSeg.h"

SVGPathSeg.h also  has #if ENABLE(SVG) inside of it. So the whole if
ENABLE(SVG) that is guarding these includes should go away and these headers
should just be sorted with the other headers (as SVGElementInstance.h was
before).


>  #endif

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.