Bug 15561 - GTK port needs DumpRenderTree implementation
Summary: GTK port needs DumpRenderTree implementation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
: 14735 (view as bug list)
Depends on: 15569
Blocks: 14725
  Show dependency treegraph
 
Reported: 2007-10-19 03:54 PDT by Eric Seidel (no email)
Modified: 2007-12-04 04:17 PST (History)
5 users (show)

See Also:


Attachments
partial patch (6.51 KB, patch)
2007-10-19 03:55 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
updated partial patch (7.91 KB, patch)
2007-10-19 20:46 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
a more complete patch (7.91 KB, patch)
2007-10-20 11:55 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
a more complete patch (previous one was missing files) (24.44 KB, patch)
2007-10-20 11:57 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
WebKitFrame changes needed to support DRT (10.74 KB, patch)
2007-11-22 23:49 PST, Alp Toker
mjs: review-
Details | Formatted Diff | Diff
WebKitFrame changes needed to support DRT, with fixed licensing (8.05 KB, patch)
2007-11-23 12:52 PST, Alp Toker
no flags Details | Formatted Diff | Diff
0001-First-stub-at-DRT-for-Gtk.patch (27.51 KB, patch)
2007-12-02 03:38 PST, Xan Lopez
alp: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-10-19 03:54:42 PDT
GTK port needs DumpRenderTree implementation

See attached patch.
Comment 1 Eric Seidel (no email) 2007-10-19 03:55:11 PDT
Created attachment 16729 [details]
partial patch
Comment 2 Holger Freyther 2007-10-19 04:57:38 PDT
Add the gtk keyword.
Comment 3 Holger Freyther 2007-10-19 04:58:18 PDT
*** Bug 14735 has been marked as a duplicate of this bug. ***
Comment 4 Holger Freyther 2007-10-19 04:59:24 PDT
Add the blocking as well...
Comment 5 Eric Seidel (no email) 2007-10-19 20:46:58 PDT
Created attachment 16738 [details]
updated partial patch
Comment 6 Eric Seidel (no email) 2007-10-20 11:55:20 PDT
Created attachment 16743 [details]
a more complete patch

Ok, I give up.  Someone else will have to finish this.
Comment 7 Eric Seidel (no email) 2007-10-20 11:57:34 PDT
Created attachment 16744 [details]
a more complete patch (previous one was missing files)
Comment 8 Alp Toker 2007-11-22 23:49:53 PST
Created attachment 17456 [details]
WebKitFrame changes needed to support DRT

This patch implements and documents more of the WebKitFrame public API.
Comment 9 Maciej Stachowiak 2007-11-23 01:22:13 PST
Comment on attachment 17456 [details]
WebKitFrame changes needed to support DRT

I'm ok with contributing to this file under LGPL assuming other WebKit/Gtk hackers are ok with this direction. However, I don't think you can just delete the BSD-style license. I think you need to preserve the notice, although of course the LGPL will take precedence.

The code changes look ok to me but I think I should r- over the license detail. Please fix and resubmit.
Comment 10 Alp Toker 2007-11-23 01:37:38 PST
(In reply to comment #9)
> (From update of attachment 17456 [details] [edit])
> I'm ok with contributing to this file under LGPL assuming other WebKit/Gtk
> hackers are ok with this direction. However, I don't think you can just delete
> the BSD-style license. I think you need to preserve the notice, although of
> course the LGPL will take precedence.

The only way you can re-license is to completely remove the old text, otherwise new code will remain optionally available under the BSD license, right?
Comment 11 Alp Toker 2007-11-23 12:52:19 PST
Created attachment 17470 [details]
WebKitFrame changes needed to support DRT, with fixed licensing

Guess we'll have to punt on the re-licensing until those issues are solved.
Comment 12 Mark Rowe (bdash) 2007-11-23 13:18:35 PST
Comment on attachment 17470 [details]
WebKitFrame changes needed to support DRT, with fixed licensing

r=me, except you should be using 0 consistently instead of both 0 and NULL.
Comment 13 Alp Toker 2007-11-27 22:11:19 PST
Comment on attachment 17470 [details]
WebKitFrame changes needed to support DRT, with fixed licensing

Patch already landed, but the bug report needs to remain open. Removing r+ flag to clear the commit queue.
Comment 14 Xan Lopez 2007-12-02 03:38:59 PST
Created attachment 17641 [details]
0001-First-stub-at-DRT-for-Gtk.patch

Patch updated to work with trunk.

Few things I had to do (also using WorkQueueItemGtk from Alp):

- Implement get_children and get_inner_text for WebKitWebFrame.
- Make JSStringCopyUTF8CString return something.
- Make ScriptItem::invoke() use script() instead of url()
- Generally update to use new APIs

And probably some other things I forget :)

Have to run now, please have a look at the patch and later I can update a new version addressing any comment and with ChangeLog etc.
-
Comment 15 Xan Lopez 2007-12-02 09:12:36 PST
Another small comment: the Qt port seems to have a class explicitely named FrameLoaderClientQt, while we have two classes named FrameLoaderClient, but in different namespaces. Is this intentional?
Comment 16 Alp Toker 2007-12-04 04:08:47 PST
Comment on attachment 17641 [details]
0001-First-stub-at-DRT-for-Gtk.patch

r=me

This patch needs some amount of style cleanup, less clobbering of the shared headers, and the new API needs to be made internal-only till discussed, but basically fine and a good start.
Comment 17 Alp Toker 2007-12-04 04:17:55 PST
Landed in r28384. Still needs lots of work.

I'll watch the build bots to make sure nothing breaks, and disable DRT in the default GTK+ build if it causes trouble.

Thanks Xan!