[Webkit-unassigned] [Bug 15409] FrameLoaderClientGtk hardcodes data, including platform to Linux i686

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Nov 5 08:11:19 PST 2007


http://bugs.webkit.org/show_bug.cgi?id=15409


alp at atoker.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16804|review?                     |review-
               Flag|                            |




------- Comment #15 from alp at atoker.com  2007-11-05 08:11 PDT -------
(From update of attachment 16804)
>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 26888)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2007-10-22  Christian Dywan  <christian at twotoasts.de>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WARNING: NO TEST CASES ADDED OR CHANGED
>+
>+        Implement systemName for use in the user agent.
>+
>+        * WebCore.pro:
>+        * platform/SystemName.cpp: Added.
>+        (WebCore::systemName):
>+        * platform/SystemName.h: Added.
>+        * platform/win/SystemNameWin.cpp: Added.
>+        (WebCore::windowsVersion):
>+        * platform/win/SystemNameWin.h: Added.
>+
> 2007-10-22  Nikolas Zimmermann  <zimmermann at kde.org>
> 
>         Reviewed by Simon.
>Index: WebCore/WebCore.pro
>===================================================================
>--- WebCore/WebCore.pro	(revision 26888)
>+++ WebCore/WebCore.pro	(working copy)
>@@ -669,6 +669,7 @@ SOURCES += \
>     platform/SharedBuffer.cpp \
>     platform/String.cpp \
>     platform/StringImpl.cpp \
>+    platform/SystemName.cpp \
>     platform/TextCodec.cpp \
>     platform/TextCodecLatin1.cpp \
>     platform/TextCodecUTF16.cpp \
>@@ -955,6 +956,9 @@ gtk-port {
>         ../WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp \
>         ../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp \
>         ../WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp
>+
>+    unix: 
>+    else: SOURCES += platform/win/SystemNameWin.cpp
> }
> 
> # ENABLE_DATABASE probably cannot be disabled without breaking things
>Index: WebCore/platform/SystemName.cpp
>===================================================================
>--- WebCore/platform/SystemName.cpp	(revision 0)
>+++ WebCore/platform/SystemName.cpp	(revision 0)
>@@ -0,0 +1,63 @@
>+/*
>+ * Copyright (C) 2007 Christian Dywan
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
>+ */
>+
>+#include "config.h"
>+#include "SystemName.h"
>+
>+#include "CString.h"
>+#include "PlatformString.h"
>+
>+#if PLATFORM(UNIX)
>+#include <sys/utsname.h>
>+#elif defined(PLATFORM(WIN))
>+#include <win/SystemNameWin.h>
>+#endif
>+
>+namespace WebCore {
>+
>+String systemName(void)
>+{
>+    #if PLATFORM(MAC)
>+        #if PLATFORM(X86)
>+        return "Intel Mac OS X";
>+        #else
>+        return "PPC Mac OS X";
>+        #endif
>+    #elif PLATFORM(UNIX)
>+    struct utsname name;
>+    if(uname(&name) != -1){
>+        return String::format("%s %s", name.sysname, name.machine);
>+    }
>+    else{
>+        return "Linux i686";
>+    }

Coding style: no braces needed here. I'm pretty sure "Linux i686" is not a good
default value here. Better to either return some kind of "Unknown" or otherwise
pass up an error.

>+    #elif PLATFORM(WIN)
>+    return windowsVersion();
>+    #else
>+    return "Unknown";
>+    #endif
>+}
>+
>+}
>Index: WebCore/platform/SystemName.h
>===================================================================
>--- WebCore/platform/SystemName.h	(revision 0)
>+++ WebCore/platform/SystemName.h	(revision 0)
>@@ -0,0 +1,37 @@
>+/*
>+ * Copyright (C) 2007 Christian Dywan
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
>+ */
>+
>+#ifndef SystemName_h
>+#define SystemName_h
>+
>+namespace WebCore {
>+
>+    class String;
>+
>+    String systemName();
>+
>+}
>+
>+#endif // SystemName_h
>Index: WebCore/platform/win/SystemNameWin.cpp
>===================================================================
>--- WebCore/platform/win/SystemNameWin.cpp	(revision 0)
>+++ WebCore/platform/win/SystemNameWin.cpp	(revision 0)
>@@ -0,0 +1,66 @@
>+/*
>+ * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
>+ * Copyright (C) 2007 Christian Dywan
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ *
>+ * 1.  Redistributions of source code must retain the above copyright
>+ *     notice, this list of conditions and the following disclaimer. 
>+ * 2.  Redistributions in binary form must reproduce the above copyright
>+ *     notice, this list of conditions and the following disclaimer in the
>+ *     documentation and/or other materials provided with the distribution. 
>+ * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
>+ *     its contributors may be used to endorse or promote products derived
>+ *     from this software without specific prior written permission. 
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
>+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
>+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
>+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
>+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+#include "SystemNameWin.h"
>+
>+#include "CString.h"
>+#include "PlatformString.h"
>+
>+#include <windowsx.h>
>+
>+namespace WebCore {
>+
>+// Copied from osVersion, WebKit/win/WebView.cpp, line 1593
>+String windowsVersion()
>+{
>+    String osVersion;
>+    OSVERSIONINFO versionInfo = {0};
>+    versionInfo.dwOSVersionInfoSize = sizeof(versionInfo);
>+    GetVersionEx(&versionInfo);
>+
>+    if (versionInfo.dwPlatformId == VER_PLATFORM_WIN32_WINDOWS) {
>+        if (versionInfo.dwMajorVersion == 4) {
>+            if (versionInfo.dwMinorVersion == 0)
>+                osVersion = "Windows 95";
>+            else if (versionInfo.dwMinorVersion == 10)
>+                osVersion = "Windows 98";
>+            else if (versionInfo.dwMinorVersion == 90)
>+                osVersion = "Windows 98; Win 9x 4.90";
>+        }
>+    } else if (versionInfo.dwPlatformId == VER_PLATFORM_WIN32_NT)
>+        osVersion = String::format("Windows NT %d.%d", versionInfo.dwMajorVersion, versionInfo.dwMinorVersion);
>+
>+    if (!osVersion.length())
>+        osVersion = String::format("Windows %d.%d", versionInfo.dwMajorVersion, versionInfo.dwMinorVersion);
>+
>+    return osVersion;
>+}
>+
>+} // namespace WebCore

Someone familiar with these issues on Windows should review this part.

>Index: WebCore/platform/win/SystemNameWin.h
>===================================================================
>--- WebCore/platform/win/SystemNameWin.h	(revision 0)
>+++ WebCore/platform/win/SystemNameWin.h	(revision 0)
>@@ -0,0 +1,35 @@
>+/*
>+ * Copyright (C) 2007 Christian Dywan
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
>+ */
>+
>+#ifndef SystemNameWin_h
>+#define SystemNameWin_h
>+
>+namespace WebCore {
>+
>+String windowsVersion(void);
>+
>+}
>+
>+#endif // SystemNameWin_h
>Index: WebKit/gtk/ChangeLog
>===================================================================
>--- WebKit/gtk/ChangeLog	(revision 26888)
>+++ WebKit/gtk/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2007-10-22  Christian Dywan  <christian at twotoasts.de>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Computer a proper user agent.
>+
>+        * ChangeLog:
>+        * WebCoreSupport/FrameLoaderClientGtk.cpp:
>+        (WebKit::composeUserAgent):
>+        (WebKit::FrameLoaderClient::FrameLoaderClient):
>+        (WebKit::FrameLoaderClient::userAgent):
>+        * WebCoreSupport/FrameLoaderClientGtk.h:
>+
> 2007-10-22  Alp Toker  <alp at atoker.com>
> 
>         Reviewed by Mark Rowe.
>Index: WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
>===================================================================
>--- WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp	(revision 26888)
>+++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp	(working copy)
>@@ -40,10 +40,12 @@
> #include "HTMLFrameElement.h"
> #include "HTMLFrameOwnerElement.h"
> #include "HTMLNames.h"
>+#include "Language.h"
> #include "MIMETypeRegistry.h"
> #include "NotImplemented.h"
> #include "PlatformString.h"
> #include "ResourceRequest.h"
>+#include "SystemName.h"
> #include "CString.h"
> #include "ProgressTracker.h"
> #include "webkitgtkpage.h"
>@@ -55,16 +57,54 @@ using namespace WebCore;
> 
> namespace WebKit {
> 
>+static String composeUserAgent(String applicationName)
>+{
>+    String operatingSystem;
>+
>+    #ifdef GDK_WINDOWING_X11
>+    operatingSystem = "X11";
>+    #elif defined(GTK_WINDOWING_WIN32)
>+    operatingSystem = "Windows";
>+    #elif defined(GTK_WINDOWING_QUARTZ)
>+    operatingSystem = "Macintosh";
>+    #elif defined(GTK_WINDOWING_DIRECTFB)
>+    operatingSystem = "DirectFB";
>+    #else
>+    operatingSystem = "Unknown";
>+    #endif

Perhaps there should be a notImplemented() in the "Unknown" case.

>+
>+    // FIXME: The WebKit version is hardcoded
>+
>+    return String::format("Mozilla/5.0 (%s; U; %s; %s) AppleWebKit/%s (KHTML, like Gecko) %s"
>+        , operatingSystem.utf8().data()
>+        , systemName().utf8().data()
>+        , defaultLanguage().utf8().data()
>+        , "523+"
>+        , applicationName.utf8().data());
>+}

We can fix the hard coded version in a later patch, this is fine for now. bdash
has some ideas here.

>+
> FrameLoaderClient::FrameLoaderClient(WebKitFrame* frame)
>     : m_frame(frame)
>     , m_firstData(false)
> {
>     ASSERT(m_frame);
>+
>+    m_applicationNameStandard = g_get_prgname();
> }
> 
> String FrameLoaderClient::userAgent(const KURL&)
> {
>-    return "Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/420+ (KHTML, like Gecko)";
>+    if(m_userAgentOverridden)
>+        return m_userAgentCustom;
>+
>+    if(!m_userAgentStandard.length()){
>+        if(m_applicationNameOverridden)
>+            m_userAgentStandard = composeUserAgent(m_applicationNameCustom);
>+        else
>+            m_userAgentStandard = composeUserAgent(m_applicationNameStandard);
>+    }
>+
>+    return m_userAgentStandard;
> }
> 
> WTF::PassRefPtr<WebCore::DocumentLoader> FrameLoaderClient::createDocumentLoader(const WebCore::ResourceRequest& request, const SubstituteData& substituteData)
>Index: WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h
>===================================================================
>--- WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h	(revision 26888)
>+++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h	(working copy)
>@@ -180,6 +180,12 @@ namespace WebKit {
>     private:
>         WebKitFrame* m_frame;
>         WebCore::ResourceResponse m_response;
>+        WebCore::String m_userAgentStandard;
>+        WebCore::String m_userAgentCustom;
>+        WebCore::String m_applicationNameStandard;
>+        WebCore::String m_applicationNameCustom;
>+        bool m_userAgentOverridden;
>+        bool m_applicationNameOverridden;
>         bool m_firstData;
>     };
> 


Can you consider splitting your patch into two parts?

I can review the GTK+ part straight away, and we can add the
platform/SystemName.h part afterwards, since that part needs wider review
particularly as ports might be able to share it.


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



More information about the webkit-unassigned mailing list