Bug 57328 – Introspection: missing signal annotations

• Status: UNCONFIRMED
• Component: WebKitGTK

[Andreia Gaita]

There are several signals on webframe and webview that are missing introspection annotations:

On webframe: hovering-over-link
on webview: should-begin-editing, should-end-editing, should-insert-node, should-insert-text, should-delete-range, should-show-delete-interface-for-element, should-change-selected-range, should-apply-style, set-scroll-adjustments

[Martin Pitt]

Created attachment 87305 [details]
add signal annotations

This patch adds annotations for all remaining signals which don't have void arguments, so that they at least get proper argument names in the gir (instead of "object", "p0", "p1", etc.).

Note that the ones added to webkitwebview.cpp are mostly just stubs which just get the signal name and data type of the arguments right. The actual description of the arguments is still (mostly) missing, as it's not easy to infer from just the signal name and type signature. In particular, I have no idea what the 4th bool argument of should-change-selected-range is about, so I added a "TODO" there -- this should probably be filled when committing the patch.

But at least I now have no stub argument names any more:

$ grep '="p0"' WebKit-1.0.gir
$

Wrt. copyright, I don't really care; hereby I agree to hand over copyright to you, and release the patch under the standard webkit licenses.

[Gustavo Noronha (kov)]

Comment on attachment 87305 [details]
add signal annotations

Thanks for the patch! To land the patch we need a ChangeLog entry you can generate with ./Tools/Scripts/prepare-ChangeLog (use --git-commit HEAD if you are using git and committed the changes). Also, you can keep your copyright, the WebKit project does not require copyright assignment, feel free to add your name to the header.

Now for comments on the content: the addition of documentation for hovering-over-link is welcome. Do we really need to add documentation to set-scroll-adjustments, since it's actually being overriden from GtkWidget? As for the other signals, they are undocumented on purpose - their design is not finished, and they only exist for internal purposes at this point (for the test runner). These signals may change, so perhaps we would need to annotate them to be ignored. Marking r- for these reasons and for lack of ChangeLog, thanks for working on this!

[Christian Dywan]

See bug 19456 for discussion on editing API. From my point of view the signals are not usable for applications in their current form.

[Andreia Gaita]

Thank you for the review, I need to thank Martin Pitt a lot for the great work in hunting down these annotations - I'm trying to get an updated webkit-sharp release out, this is very helpful to get a reliable binding going. The signals that should be ignored are all those listed on the bug mentioned on comment #3, right? Any others I should be mindful of?

[Gustavo Noronha (kov)]

(In reply to comment #4)
> Thank you for the review, I need to thank Martin Pitt a lot for the great work in hunting down these annotations - I'm trying to get an updated webkit-sharp release out, this is very helpful to get a reliable binding going. The signals that should be ignored are all those listed on the bug mentioned on comment #3, right? Any others I should be mindful of?

In general we've been very strict about documenting new API, so if something is not documented it is either very old, or not intended for public usage as of yet. If in doubt feel free to ping me, mrobinson, xan or anyone else who's available in #webkit-gtk@FreeNode and we'll be happy to help =)