Summary: | [Gtk] Implement dumpFrameScrollPosition in DumpRenderTree | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||||||
Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | mrobinson, pnormand | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Zan Dobersek
2012-06-18 09:56:45 PDT
Created attachment 148115 [details]
Patch
Comment on attachment 148115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148115&action=review > Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:140 > + if (abs(x) > 0 || abs(y) > 0) { > + if (webkit_web_frame_get_parent(frame)) > + printf("frame '%s' ", webkit_web_frame_get_name(frame)); > + printf("scrolled to %d,%d\n", x, y); > + } > + > + if (gLayoutTestController->dumpChildFrameScrollPositions()) { > + GSList* children = DumpRenderTreeSupportGtk::getFrameChildren(frame); > + for (GSList* child = children; child; child = g_slist_next(child)) > + dumpFrameScrollPosition(static_cast<WebKitWebFrame*>(child->data)); > + g_slist_free(children); > + } Hrm. Isn't it possible to get this with the DOM bindings instead of using DumpRenderTreeSupport? Created attachment 148273 [details]
Patch
Sorry for the new API! :>
Comment on attachment 148273 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148273&action=review Looks good to me! We just need another GTK+ reviewer to approve the API. > Source/WebKit/gtk/webkit/webkitwebframe.cpp:1188 > + * Returns: (transfer none): the #WebKitDOMDocument currently loaded in the @frame Should say "or %NULL if no document is loaded." > Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:136 > + glong x, y; > + x = webkit_dom_dom_window_get_page_x_offset(domWindow); > + y = webkit_dom_dom_window_get_page_y_offset(domWindow); Just collapse these into glong x = webkit_dom_dom_window_get_page_x_offset(domWindow); glong y = webkit_dom_dom_window_get_page_y_offset(domWindow); Created attachment 148294 [details]
Patch
Comment on attachment 148294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148294&action=review > Source/WebKit/gtk/webkit/webkitwebframe.cpp:1191 > + * Since: 2.0 We are not sure yet, whether we will release 2.0 or 1.10, so I'd use 1.10 for now just in case. > Source/WebKit/gtk/webkit/webkitwebframe.cpp:1194 > +webkit_web_frame_get_dom_document(WebKitWebFrame* frame) Maybe we could also update webkit_web_view_get_dom_document) to use this method now with priv->mainFrame. Comment on attachment 148294 [details]
Patch
New API LGTM for the record
Created attachment 151292 [details]
Patch
Comment on attachment 151292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151292&action=review > Source/WebKit/gtk/webkit/webkitwebframe.cpp:1194 > +WebKitDOMDocument* > +webkit_web_frame_get_dom_document(WebKitWebFrame* frame) One nit here is that breaking this line doesn't follow WebKit style. (In reply to comment #9) > (From update of attachment 151292 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=151292&action=review > > > Source/WebKit/gtk/webkit/webkitwebframe.cpp:1194 > > +WebKitDOMDocument* > > +webkit_web_frame_get_dom_document(WebKitWebFrame* frame) > > One nit here is that breaking this line doesn't follow WebKit style. Fixed and committed in r122147 http://trac.webkit.org/changeset/122147 |