Bug 12160 - Linux/Gdk build fixes
Summary: Linux/Gdk build fixes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 420+
Hardware: PC OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-08 01:35 PST by Krzysztof Kowalczyk
Modified: 2007-02-08 23:02 PST (History)
0 users

See Also:


Attachments
linux\gdk build fixes (108.05 KB, patch)
2007-01-08 01:38 PST, Krzysztof Kowalczyk
no flags Details | Formatted Diff | Diff
Updated linux/gtk patch to r18755 (124.92 KB, patch)
2007-01-10 21:04 PST, Krzysztof Kowalczyk
no flags Details | Formatted Diff | Diff
Updated linux/gdk fixes to r18930 (131.03 KB, patch)
2007-01-17 23:34 PST, Krzysztof Kowalczyk
no flags Details | Formatted Diff | Diff
linux\gdk build fixes (111.97 KB, patch)
2007-01-31 15:45 PST, Krzysztof Kowalczyk
no flags Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero uno (32.36 KB, patch)
2007-02-06 20:31 PST, Krzysztof Kowalczyk
aroben: review+
Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero dos (8.55 KB, patch)
2007-02-06 20:34 PST, Krzysztof Kowalczyk
aroben: review+
Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero treis (17.26 KB, patch)
2007-02-06 20:39 PST, Krzysztof Kowalczyk
aroben: review+
Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero quatro (4.21 KB, patch)
2007-02-06 20:43 PST, Krzysztof Kowalczyk
mjs: review+
Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero cinco (3.90 KB, patch)
2007-02-06 20:47 PST, Krzysztof Kowalczyk
aroben: review+
Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero seis (1.13 KB, patch)
2007-02-06 20:51 PST, Krzysztof Kowalczyk
mjs: review+
Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero siete (5.32 KB, patch)
2007-02-06 20:57 PST, Krzysztof Kowalczyk
mjs: review+
Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero huit (16.36 KB, patch)
2007-02-07 16:42 PST, Krzysztof Kowalczyk
aroben: review+
Details | Formatted Diff | Diff
Linux\gdk build fixes - el patch numero nove (8.36 KB, patch)
2007-02-07 16:54 PST, Krzysztof Kowalczyk
mjs: review+
Details | Formatted Diff | Diff
linux\gdk build fixes for cairo (10.59 KB, patch)
2007-02-07 17:12 PST, Krzysztof Kowalczyk
aroben: review+
Details | Formatted Diff | Diff
ScrollView for gdk update (2.67 KB, patch)
2007-02-07 17:44 PST, Krzysztof Kowalczyk
mjs: review+
Details | Formatted Diff | Diff
linux\gdk build fixes for EditorClientGdk and FrameGdk (79.81 KB, patch)
2007-02-07 23:56 PST, Krzysztof Kowalczyk
aroben: review+
Details | Formatted Diff | Diff
linux\gdk build fixes - update bakefiles (11.85 KB, patch)
2007-02-08 00:01 PST, Krzysztof Kowalczyk
aroben: review+
Details | Formatted Diff | Diff
make keyboard link-walking work on gdk (1.08 KB, patch)
2007-02-08 15:22 PST, Krzysztof Kowalczyk
no flags Details | Formatted Diff | Diff
fix keyboard link walking; bakefile fix (1.75 KB, patch)
2007-02-08 16:37 PST, Krzysztof Kowalczyk
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Krzysztof Kowalczyk 2007-01-08 01:35:28 PST
Linux/Gdk build fixes.
Comment 1 Krzysztof Kowalczyk 2007-01-08 01:38:58 PST
Created attachment 12297 [details]
linux\gdk build fixes
Comment 2 Krzysztof Kowalczyk 2007-01-10 21:04:55 PST
Created attachment 12354 [details]
Updated linux/gtk patch to r18755
Comment 3 Krzysztof Kowalczyk 2007-01-17 23:34:22 PST
Created attachment 12527 [details]
Updated linux/gdk fixes to r18930
Comment 4 Krzysztof Kowalczyk 2007-01-31 15:45:35 PST
Created attachment 12835 [details]
linux\gdk build fixes
Comment 5 Krzysztof Kowalczyk 2007-02-06 20:31:33 PST
Created attachment 12989 [details]
Linux\gdk build fixes - el patch numero uno

First in a series of small patches fixing gdk/linux build.
Comment 6 Krzysztof Kowalczyk 2007-02-06 20:34:56 PST
Created attachment 12990 [details]
Linux\gdk build fixes - el patch numero dos

Second in a series of small patches for fixing linux\gdk build.
Comment 7 Krzysztof Kowalczyk 2007-02-06 20:39:09 PST
Created attachment 12991 [details]
Linux\gdk build fixes - el patch numero treis

Third in a series of small patches for fixing linux\gdk build. Those are related to graphics.
Comment 8 Krzysztof Kowalczyk 2007-02-06 20:43:45 PST
Created attachment 12992 [details]
Linux\gdk build fixes - el patch numero quatro

Fourth in a series of small patches for fixing linux/gdk build - those are related to font handling.
Comment 9 Krzysztof Kowalczyk 2007-02-06 20:47:05 PST
Created attachment 12993 [details]
Linux\gdk build fixes - el patch numero cinco

Fifth in a series of small patches for fixing linux/gdk build - add DragDataGdk.cpp.
Comment 10 Krzysztof Kowalczyk 2007-02-06 20:51:26 PST
Created attachment 12994 [details]
Linux\gdk build fixes - el patch numero seis

Fix crash on gdk build due to mismatched type of alloc/free function (allocs with fastAlloc() but frees with regular free())
Comment 11 Krzysztof Kowalczyk 2007-02-06 20:57:40 PST
Created attachment 12995 [details]
Linux\gdk build fixes - el patch numero siete

Seventh in a series of small patches for fixing linux/gdk build - functions related to networking.
Comment 12 Maciej Stachowiak 2007-02-07 01:39:29 PST
Krzysztof: thanks for breaking it up! You are my hero.
Comment 13 Maciej Stachowiak 2007-02-07 01:42:25 PST
Comment on attachment 12992 [details]
Linux\gdk build fixes - el patch numero quatro

r=me
Comment 14 Adam Roben (:aroben) 2007-02-07 01:43:07 PST
Comment on attachment 12989 [details]
Linux\gdk build fixes - el patch numero uno

+    return "Mozilla/5.0 (PC; U; Intel; Windows; en) AppleWebKit/420+ (KHTML, like Gecko)";

   "Windows" doesn't seem completely accurate here...

   It'd be nice to see this as an svn cp rather than just an svn add.

   r=me
Comment 15 Adam Roben (:aroben) 2007-02-07 01:45:22 PST
Comment on attachment 12990 [details]
Linux\gdk build fixes - el patch numero dos

+namespace WebCore
+{

   The opening brace should be on the same line as namespace WebCore.

+    // FIXME: this method always returns true
+    notImplemented();
+    return false;

   The comment here seems completely incorrect.

+bool EventHandler::handleDrag(const MouseEventWithHitTestResults& event)
+{
+    //notImplemented();
+    return false;
+}

   Why is notImplented() commented out?

   Again, svn cp would be better.

   r=me.
Comment 16 Adam Roben (:aroben) 2007-02-07 02:01:03 PST
Comment on attachment 12993 [details]
Linux\gdk build fixes - el patch numero cinco

r=me
Comment 17 Maciej Stachowiak 2007-02-07 02:02:09 PST
Comment on attachment 12993 [details]
Linux\gdk build fixes - el patch numero cinco

r=me
Comment 18 Maciej Stachowiak 2007-02-07 02:02:20 PST
Comment on attachment 12994 [details]
Linux\gdk build fixes - el patch numero seis

r=me
Comment 19 Maciej Stachowiak 2007-02-07 02:03:39 PST
Comment on attachment 12995 [details]
Linux\gdk build fixes - el patch numero siete

I'm not sure ResourceHandleManager::self() is a good name. get() was kind of vague, but a more typical name for a static method when applying the singleton pattern like this would be sharedInstance() or just shared(). Otherwise these changes look fine. r=me and feel free to adjust name when landing if you want.
Comment 20 Adam Roben (:aroben) 2007-02-07 10:09:25 PST
Comment on attachment 12991 [details]
Linux\gdk build fixes - el patch numero treis

+#include <cairo.h>
 #include "FloatRect.h"
 #include "Font.h"
+#include "FontData.h"
 #include "IntRect.h"
-#include <cairo.h>
 #include <math.h>
+#include <stdio.h>

   cairo.h should stay where it was, above math.h.

-void GraphicsContext::setFocusRingClip(const IntRect&)
+void GraphicsContext::setFocusRingClip(const IntRect& rect)

   This change is unnecessary.

+void GraphicsContext::setPlatformFillColor(const Color& col)
+{
+    setColor(m_data->context, col);
+    cairo_fill(m_data->context);

   Does cairo_fill actually do a fill? That's not what we want here, we just want to set a color for when a fill does happen later. Ditto for stroke.

+    if (m_decoder) {
+        delete  m_decoder;
+        m_decoder = 0;

   Null check is unnecessary. There's an extra space after 'delete'.

+    BitmapImage* img = new BitmapImage();

   Parentheses are unnecessary here.

   If cairo_stroke and cairo_fill don't actually do a stroke/fill, r=me.
Comment 21 Krzysztof Kowalczyk 2007-02-07 13:07:47 PST
Comment on attachment 12992 [details]
Linux\gdk build fixes - el patch numero quatro

commited
Comment 22 Krzysztof Kowalczyk 2007-02-07 13:45:59 PST
Comment on attachment 12989 [details]
Linux\gdk build fixes - el patch numero uno

commited. Used svn cp and changed user agent to "Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/420+ (KHTML, like Gecko)" as per review comments.
Comment 23 Krzysztof Kowalczyk 2007-02-07 14:04:54 PST
Comment on attachment 12990 [details]
Linux\gdk build fixes - el patch numero dos

committed, used svn cp, fixed the brace placement and removed bad comment (it's already gone in qt version), as per review comments.
Comment 24 Krzysztof Kowalczyk 2007-02-07 14:21:16 PST
Comment on attachment 12993 [details]
Linux\gdk build fixes - el patch numero cinco

commited
Comment 25 Krzysztof Kowalczyk 2007-02-07 14:26:32 PST
Comment on attachment 12994 [details]
Linux\gdk build fixes - el patch numero seis

committed
Comment 26 Krzysztof Kowalczyk 2007-02-07 14:45:13 PST
Comment on attachment 12995 [details]
Linux\gdk build fixes - el patch numero siete

commited. used sharedInstance() instead of self() as per review comments.
Comment 27 Krzysztof Kowalczyk 2007-02-07 15:20:30 PST
Comment on attachment 12991 [details]
Linux\gdk build fixes - el patch numero treis

committed ImageCairo.cpp, ImageSoruceCairo.cpp and ImageGdk.cpp making changes suggest in review. Will re-submit GraphicsContextCairo.cpp since Adam is right about cairo_fill and cairo_stroke not being used correctly.
Comment 28 Krzysztof Kowalczyk 2007-02-07 16:42:04 PST
Created attachment 13022 [details]
Linux\gdk build fixes - el patch numero huit

Eight in a series of small patches for fixing linux/gdk build.
Comment 29 Krzysztof Kowalczyk 2007-02-07 16:54:24 PST
Created attachment 13023 [details]
Linux\gdk build fixes - el patch numero nove

Ninth in the series of small patches for fixing linux/gdk build.
Comment 30 Krzysztof Kowalczyk 2007-02-07 17:12:50 PST
Created attachment 13026 [details]
linux\gdk build fixes for cairo

10th patch for linux/gdk for cairo-related stuff. The difference between those patches and the ones submitted earlier are:
* in GraphicsContext.cpp - remove functions instead of #ifdefing them out for all platforms
* in GraphicsContextCairo.cpp make setPlatformFillColor() and setPlatformStrokeColor() no-ops since Adam was right that cairo_fill and cairo_stroke shouldn't be used there (they do the actual drawing)
Comment 31 Krzysztof Kowalczyk 2007-02-07 17:44:07 PST
Created attachment 13028 [details]
ScrollView for gdk update

Add ScrollView::updateGeometry() for gdk.
Comment 32 Maciej Stachowiak 2007-02-07 18:27:07 PST
Comment on attachment 13028 [details]
ScrollView for gdk update

r=me
Comment 33 Maciej Stachowiak 2007-02-07 18:33:22 PST
Comment on attachment 13023 [details]
Linux\gdk build fixes - el patch numero nove

r=me
Comment 34 Krzysztof Kowalczyk 2007-02-07 22:35:12 PST
Comment on attachment 13023 [details]
Linux\gdk build fixes - el patch numero nove

commited
Comment 35 Krzysztof Kowalczyk 2007-02-07 22:39:24 PST
Comment on attachment 13028 [details]
ScrollView for gdk update

commited
Comment 36 Krzysztof Kowalczyk 2007-02-07 23:56:11 PST
Created attachment 13046 [details]
linux\gdk build fixes for EditorClientGdk and FrameGdk

linux\gdk build fixes for EditorClientGdk and FrameGdk
Comment 37 Krzysztof Kowalczyk 2007-02-08 00:01:15 PST
Created attachment 13047 [details]
linux\gdk build fixes - update bakefiles

linux\gdk build fixes - update bakefiles
Comment 38 Adam Roben (:aroben) 2007-02-08 01:59:26 PST
Comment on attachment 13047 [details]
linux\gdk build fixes - update bakefiles

         DerivedSources/WebCore/JSHTMLQuoteElement.cpp
+        DerivedSources/WebCore/JSHTMLSelectElement.cpp
         DerivedSources/WebCore/JSHTMLScriptElement.cpp

   Select should go below Script.

   r=me
Comment 39 Adam Roben (:aroben) 2007-02-08 02:01:08 PST
Comment on attachment 13026 [details]
linux\gdk build fixes for cairo

r=me
Comment 40 Adam Roben (:aroben) 2007-02-08 02:05:52 PST
Comment on attachment 13022 [details]
Linux\gdk build fixes - el patch numero huit

 #include "ChromeClient.h"
+#include "Shared.h"

   Shared.h shouldn't be necessary anymore.

+    // FIXME: optimize the way CursorQt is optmized: only one copy of a given
+    // cursor type
+    static Cursor c = gdk_cursor_new(GDK_LEFT_PTR);

   I think you've already implemented this FIXME. Since c is static gdk_cursor_new will only be called once.

 #include <assert.h>
 #include <gdk/gdk.h>
+#include "SystemTime.h"

   SystemTime.h should go above <assert.h>

+    m_shiftKey = event->button.state & GDK_SHIFT_MASK != 0;
+    m_ctrlKey = event->button.state & GDK_CONTROL_MASK != 0;
+    m_altKey = event->button.state & GDK_MOD1_MASK != 0;
+    m_metaKey = event->button.state & GDK_MOD2_MASK != 0;

   The '!= 0' is redundant in each of these cases.

   r=me.
Comment 41 Adam Roben (:aroben) 2007-02-08 12:45:09 PST
Comment on attachment 13046 [details]
linux\gdk build fixes for EditorClientGdk and FrameGdk

+#ifndef EditorClientGtk_H
+#define EditorClientGtk_H

   Sould be EditorClientGdk_h (t -> d and lowercase h)

+#include <gdk/gdk.h>
+#include <gtk/gtk.h>

   These headers should go at the end of the list.

+#include <gdk/gdk.h>
+#include <gtk/gtk.h>

   Ditto (in main.cpp)

   r=me.
Comment 42 Krzysztof Kowalczyk 2007-02-08 14:18:21 PST
Comment on attachment 13047 [details]
linux\gdk build fixes - update bakefiles

commited. Corrected position of JSHTMLSelectElement.cpp as per review comments.
Comment 43 Krzysztof Kowalczyk 2007-02-08 14:26:52 PST
Comment on attachment 13026 [details]
linux\gdk build fixes for cairo

commited. Added one last-minute change: a notImplemented() macro for MSVC as well (MSVC doesn't understand __PRETTY_FUNCTION__) since windows port also uses GraphicsContextCairo.cpp
Comment 44 Krzysztof Kowalczyk 2007-02-08 14:50:13 PST
Comment on attachment 13022 [details]
Linux\gdk build fixes - el patch numero huit

commited. Removed <Shared.h>, re-arranged headers order, removed redundant != 0. FIXME comment refers to the fact that there are several static variables for the same GDK_LEFT_PTR cursor, which could be collapsed into just one, so it's still valid. One last-minute change: adding (notImplemented()) ChromeClientGdk::shouldInterruptJavaScript() (it was added after the patch has been submitted)
Comment 45 Krzysztof Kowalczyk 2007-02-08 15:08:57 PST
Comment on attachment 13046 [details]
linux\gdk build fixes for EditorClientGdk and FrameGdk

commited. Fix header guard name and re-arranged include order as per review comments.
Comment 46 Krzysztof Kowalczyk 2007-02-08 15:22:30 PST
Created attachment 13068 [details]
make keyboard link-walking work on gdk

This is the last one in this batch, I promise. Makes gdk's tab key recognized as tab so that keyboard link walking works.
Comment 47 Krzysztof Kowalczyk 2007-02-08 16:37:40 PST
Created attachment 13074 [details]
fix keyboard link walking; bakefile fix

Make gdk's tab key recognized as tab to fix keyboard link walking. bakefile update for file renaming.
Comment 48 Darin Adler 2007-02-08 20:55:36 PST
Comment on attachment 13074 [details]
fix keyboard link walking; bakefile fix

r=me