[webkit-reviews] review denied: [Bug 111578] Web Inspector: split Console into two entities, a web-facing bound object and page console. : [Attachment 193448] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 18 06:43:56 PDT 2013


Vsevolod Vlasov <vsevik at chromium.org> has denied Sergey Ryazanov
<serya at chromium.org>'s request for review:
Bug 111578: Web Inspector: split Console into two entities, a web-facing bound
object and page console.
https://bugs.webkit.org/show_bug.cgi?id=111578

Attachment 193448: Patch
https://bugs.webkit.org/attachment.cgi?id=193448&action=review

------- Additional Comments from Vsevolod Vlasov <vsevik at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193448&action=review


> Source/WebCore/ChangeLog:10
> +	   No new tests (OOPS!).

Please remove this line, otherwise commit queue won't let you commit the patch.


> Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp:-80
> -	       // FIXME: Pass actual line number and source URL.

Please keep FIXME here.

> Source/WebCore/dom/Document.cpp:127
> +#include "PageConsole.h"

Do we still need to include Console.h here?

> Source/WebCore/dom/Document.cpp:4848
> +    Page* p = page();

Page* page = ...

> Source/WebCore/dom/Document.cpp:4861
> +    if (p)

Ditto

> Source/WebCore/page/Console.h:81
>      static void mute();

Should be removed.

> Source/WebCore/page/Console.h:82
>      static void unmute();

Ditto

> Source/WebCore/page/Console.h:-99
> -    inline Page* page() const;

Why did you remove this?

> Source/WebCore/xml/XSLStyleSheetLibxslt.cpp:27
>  #include "DOMWindow.h"

Could be removed now.

> Source/WebCore/xml/XSLTProcessorLibxslt.cpp:30
>  #include "DOMWindow.h"

Could be removed now.


More information about the webkit-reviews mailing list