Bug 139201 – [GTK] Missing API detected in GObject DOM bindings after r176630

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

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

[Dean Jackson]

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.

[Dean Jackson]

Created attachment 242484 [details]
Patch

[Carlos Garcia Campos]

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?

[Dean Jackson]

(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.

[Dean Jackson]

(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 :(

[Dean Jackson]

Created attachment 242499 [details]
Patch

[Dean Jackson]

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 :(

[Carlos Garcia Campos]

(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

[Carlos Garcia Campos]

Created attachment 242557 [details]
Patch

[Carlos Garcia Campos]

Created attachment 242558 [details]
Try to fix GTK build

I wonder why it built for me

[Carlos Garcia Campos]

EWS failure must be caused by previous patches, it builds fine here

[Carlos Garcia Campos]

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.

[Martin Robinson]

(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.

[Carlos Garcia Campos]

Created attachment 242626 [details]
Updated patch

Changed the GENERATE_BINDINGS, let's see whether this works

[Gustavo Noronha (kov)]

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
> [phone]  Carlos Garcia Campos  <[e-mail]>

Duplicate ChangeLog entry.

[Carlos Garcia Campos]

(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
> > [phone]  Carlos Garcia Campos  <[e-mail]>
> 
> Duplicate ChangeLog entry.

Oops

[Carlos Garcia Campos]

Committed r176919: <http://trac.webkit.org/changeset/176919>