Bug 24496 - ChromeClients don't get addMessageToConsole() called for non-string arguments
Summary: ChromeClients don't get addMessageToConsole() called for non-string arguments
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2009-03-10 11:52 PDT by Simon Fraser (smfr)
Modified: 2009-03-12 14:12 PDT (History)
3 users (show)

See Also:


Attachments
Patch, testcase, changelog (4.76 KB, patch)
2009-03-10 12:06 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Fix logging of non-string values, v2 (5.75 KB, patch)
2009-03-11 16:37 PDT, Dimitri Glazkov (Google)
simon.fraser: review+
Details | Formatted Diff | Diff
WebCore: (5.10 KB, text/plain)
2009-03-12 13:57 PDT, Dimitri Glazkov (Google)
no flags Details
2009-03-12 Dimitri Glazkov <dglazkov@chromium.org> (2.08 KB, patch)
2009-03-12 13:57 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff
Do the same for V8 bindings. (1.96 KB, patch)
2009-03-12 13:57 PDT, Dimitri Glazkov (Google)
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2009-03-10 11:52:57 PDT
Code in Console.cpp only sends messages to the chrome client if the first argument is a string. This is apparently a regression.

Note that the inspector gets all messages, because it doesn't test whether something is a string.
Comment 1 Simon Fraser (smfr) 2009-03-10 12:02:53 PDT
Probably regressed by bug 22631.
Comment 2 Simon Fraser (smfr) 2009-03-10 12:06:19 PDT
Created attachment 28445 [details]
Patch, testcase, changelog
Comment 3 Simon Fraser (smfr) 2009-03-10 12:22:37 PDT
dglakzov said that FrameLoader::executeIfJavaScriptURL () may have a similar issue
Comment 4 Dimitri Glazkov (Google) 2009-03-10 12:26:23 PDT
This will also break Chromium build, because it exposes JSValue. Perhaps we could change ScriptValue::getString() to ScriptValue::toString() which does the conversion? This should address the issue with FrameLoader::executeIfJavaScriptURL and Console::addMessage as well.

+fishd as he wrote ScriptValue originally.
Comment 5 Dimitri Glazkov (Google) 2009-03-11 16:01:39 PDT
If you don't mind, I'll reuse your awesome layout test.
Comment 6 Dimitri Glazkov (Google) 2009-03-11 16:37:46 PDT
Created attachment 28502 [details]
Fix logging of non-string values, v2

 LayoutTests/ChangeLog                              |   10 ++++++++++
 .../fast/js/console-non-string-values-expected.txt |    7 +++++++
 LayoutTests/fast/js/console-non-string-values.html |   18 ++++++++++++++++++
 WebCore/ChangeLog                                  |   19 +++++++++++++++++++
 WebCore/bindings/js/ScriptValue.cpp                |    1 -
 WebCore/bindings/js/ScriptValue.h                  |    3 +++
 WebCore/page/Console.cpp                           |    9 +++++----
 7 files changed, 62 insertions(+), 5 deletions(-)
Comment 7 Darin Adler 2009-03-12 11:50:10 PDT
<rdar://problem/6453834>
Comment 8 Dimitri Glazkov (Google) 2009-03-12 12:34:53 PDT
Landed as http://trac.webkit.org/changeset/41640.
Comment 9 Dimitri Glazkov (Google) 2009-03-12 13:57:34 PDT
Created attachment 28550 [details]
WebCore:


2009-03-12  Peter Kasting  <pkasting@google.com>

        Reviewed by Darin Fisher.

        https://bugs.webkit.org/show_bug.cgi?id=24502
        Make horizontal scrolling on Windows always go the correct direction.

        * platform/PlatformWheelEvent.h:
        * platform/win/WheelEventWin.cpp:
        (WebCore::PlatformWheelEvent::PlatformWheelEvent):

WebKit/win:

2009-03-12  Peter Kasting  <pkasting@google.com>

        Reviewed by Darin Fisher.

        https://bugs.webkit.org/show_bug.cgi?id=24502
        Make horizontal scrolling on Windows always go the correct direction.

        * WebView.cpp:
        (WebView::mouseWheel):
        (WebViewWndProc):
        * WebView.h:



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@41642 268f45cc-cd09-0410-ab3c-d52691b4dbfc
---
 WebCore/ChangeLog                      |   11 +++++++++++
 WebCore/platform/PlatformWheelEvent.h  |    2 +-
 WebCore/platform/win/WheelEventWin.cpp |    6 ++++--
 WebKit/win/ChangeLog                   |   12 ++++++++++++
 WebKit/win/WebView.cpp                 |    6 +++---
 WebKit/win/WebView.h                   |    2 +-
 6 files changed, 32 insertions(+), 7 deletions(-)
Comment 10 Dimitri Glazkov (Google) 2009-03-12 13:57:36 PDT
Created attachment 28551 [details]
2009-03-12  Dimitri Glazkov  <dglazkov@chromium.org>


        Reviewed by Adam Treat.

        https://bugs.webkit.org/show_bug.cgi?id=24525
        REGRESSION: Inspector window doesn't close when inspected page is
        destroyed. This is a revert of r41158, which became unnecessary when
        InspectorController became ref-counted in r41462.

        * inspector/InspectorController.cpp:
        (WebCore::InspectorController::inspectedPageDestroyed): Reset m_inspectedPage
            after calling close().
        (WebCore::InspectorController::stopUserInitiatedProfiling): Remove
            m_inspectedPage check guard around profile logic.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@41643 268f45cc-cd09-0410-ab3c-d52691b4dbfc
---
 WebCore/ChangeLog                         |   15 +++++++++++++++
 WebCore/inspector/InspectorController.cpp |   14 ++++++--------
 2 files changed, 21 insertions(+), 8 deletions(-)
Comment 11 Dimitri Glazkov (Google) 2009-03-12 13:57:38 PDT
Created attachment 28552 [details]
Do the same for V8 bindings.

 WebCore/ChangeLog                   |   15 +++++++++++++++
 WebCore/bindings/v8/ScriptValue.cpp |    5 +++++
 WebCore/bindings/v8/ScriptValue.h   |    6 ++++--
 3 files changed, 24 insertions(+), 2 deletions(-)
Comment 12 Dimitri Glazkov (Google) 2009-03-12 13:58:52 PDT
I should do the same changes for V8 bindings. doh.
Comment 13 Darin Fisher (:fishd, Google) 2009-03-12 14:05:30 PDT
Comment on attachment 28552 [details]
Do the same for V8 bindings.

LGTM
Comment 14 Dimitri Glazkov (Google) 2009-03-12 14:12:06 PDT
V8 changes landed as http://trac.webkit.org/changeset/41649.