Bug 139201 - [GTK] Missing API detected in GObject DOM bindings after r176630
Summary: [GTK] Missing API detected in GObject DOM bindings after r176630
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-02 14:43 PST by Carlos Alberto Lopez Perez
Modified: 2014-12-07 02:02 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.78 KB, patch)
2014-12-03 02:56 PST, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (3.78 KB, patch)
2014-12-03 10:40 PST, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (12.75 KB, patch)
2014-12-04 01:03 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Try to fix GTK build (12.78 KB, patch)
2014-12-04 01:12 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (19.98 KB, patch)
2014-12-05 04:22 PST, Carlos Garcia Campos
gns: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2014-12-02 14:43:12 PST
Since r176630 <http://trac.webkit.org/176630> the GTK WebKit GObject DOM bindings API break tests are failing.

The output is:

Missing API (API break!) detected in GObject DOM bindings
    void webkit_dom_html_element_set_inner_html(WebKitDOMHTMLElement*, const gchar*, GError**)
    gchar* webkit_dom_html_element_get_outer_html(WebKitDOMHTMLElement*)
    gchar* webkit_dom_html_element_get_inner_html(WebKitDOMHTMLElement*)
    void webkit_dom_html_element_set_outer_html(WebKitDOMHTMLElement*, const gchar*, GError**)

Re-add the missing API and rerun the ./Tools/gtk/check-for-webkitdom-api-breaks.

https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/4547/steps/WebKit%20GObject%20DOM%20bindings%20API%20break%20tests/logs/stdio
Comment 1 Dean Jackson 2014-12-02 16:42:57 PST
Yeah, sorry. I should have noticed this in review.

We should add innerHTML back onto HTMLElement and have it call up to the Element implementation.
Comment 2 Dean Jackson 2014-12-03 02:56:04 PST
Created attachment 242484 [details]
Patch
Comment 3 Carlos Garcia Campos 2014-12-03 05:14:17 PST
Instead of exposing it in both classes, we could add custom compatible API to the GObject DOM bindings. Or is that also needed for Objc bindings?
Comment 4 Dean Jackson 2014-12-03 09:55:53 PST
(In reply to comment #3)
> Instead of exposing it in both classes, we could add custom compatible API
> to the GObject DOM bindings. Or is that also needed for Objc bindings?

Yeah, that might be easier. I think we could do the same in the ObjC bindings. I'll take a look.
Comment 5 Dean Jackson 2014-12-03 10:38:45 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Instead of exposing it in both classes, we could add custom compatible API
> > to the GObject DOM bindings. Or is that also needed for Objc bindings?
> 
> Yeah, that might be easier. I think we could do the same in the ObjC
> bindings. I'll take a look.

Actually I'm not so sure how to do this for ObjC yet :(
Comment 6 Dean Jackson 2014-12-03 10:40:46 PST
Created attachment 242499 [details]
Patch
Comment 7 Dean Jackson 2014-12-03 12:44:46 PST
Weinig suggested that we follow cgarcia's advice and just add it to the GObject DOM bindings, but I'm not sure how to do that.

Sorry, but I'll leave it to you guys :(
Comment 8 Carlos Garcia Campos 2014-12-03 23:27:20 PST
(In reply to comment #7)
> Weinig suggested that we follow cgarcia's advice and just add it to the
> GObject DOM bindings, but I'm not sure how to do that.
> 
> Sorry, but I'll leave it to you guys :(

Don't worry, I'll do it
Comment 9 Carlos Garcia Campos 2014-12-04 01:03:45 PST
Created attachment 242557 [details]
Patch
Comment 10 Carlos Garcia Campos 2014-12-04 01:12:20 PST
Created attachment 242558 [details]
Try to fix GTK build

I wonder why it built for me
Comment 11 Carlos Garcia Campos 2014-12-04 01:59:20 PST
EWS failure must be caused by previous patches, it builds fine here
Comment 12 Carlos Garcia Campos 2014-12-04 23:39:16 PST
I know what the problem is with EWS. The new symbols in Element were added as unstable API by default, and the patch is now moving them as stable, but changes in the symbol file don't cause the bindings to be regenerated, so the symbols are still in WebKitDOMElementUnstable.h. The ideal solution would be to make the symbols file a dependency of the dom bindings generation, but bindings are generated in a common macro, so I'm not sure how to do that. A clean build would also fix the problem, of course.
Comment 13 Martin Robinson 2014-12-05 00:38:22 PST
(In reply to comment #12)
> I know what the problem is with EWS. The new symbols in Element were added
> as unstable API by default, and the patch is now moving them as stable, but
> changes in the symbol file don't cause the bindings to be regenerated, so
> the symbols are still in WebKitDOMElementUnstable.h. The ideal solution
> would be to make the symbols file a dependency of the dom bindings
> generation, but bindings are generated in a common macro, so I'm not sure
> how to do that. A clean build would also fix the problem, of course.

One option is to add an extra argument to GENERATE_BINDINGS which enables you to specify extra dependencies for all of the targets.
Comment 14 Carlos Garcia Campos 2014-12-05 04:22:01 PST
Created attachment 242626 [details]
Updated patch

Changed the GENERATE_BINDINGS, let's see whether this works
Comment 15 Gustavo Noronha (kov) 2014-12-06 08:20:49 PST
Comment on attachment 242626 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=242626&action=review

OK =)

> Source/WebCore/ChangeLog:24
> +2014-12-04  Carlos Garcia Campos  <cgarcia@igalia.com>

Duplicate ChangeLog entry.
Comment 16 Carlos Garcia Campos 2014-12-06 08:22:53 PST
(In reply to comment #15)
> Comment on attachment 242626 [details]
> Updated patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242626&action=review
> 
> OK =)

Thanks

> > Source/WebCore/ChangeLog:24
> > +2014-12-04  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
> Duplicate ChangeLog entry.

Oops
Comment 17 Carlos Garcia Campos 2014-12-07 02:02:27 PST
Committed r176919: <http://trac.webkit.org/changeset/176919>