[GTK] DOM objects created wrapping a base class have incorrect GObject type

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

Some of the DOM methods can create a concrete instance of another object, but return a base class. For example HTMLTableElement::insertRow() creates a HTMLTableRowElement but it's returned as a HTMLElement. Our bindings provide custom kit implementations for Node, Element, Event and EvenTarget, so in this example, we create a WebKitDOMHTMLElement, but wrapping a HTMLTableRowElement. This particular case is fixed in bug #111714 by providing a custom kit implementation for HTMLElement too, but the problem is generic and affects to all polymorphic objects that could be created by wrapping any of their types.

[Carlos Garcia Campos]

Created attachment 194712 [details]
Patch

[WebKit Review Bot]

Comment on attachment 194712 [details]
Patch

Attachment 194712 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17220806

New failing tests:
fast/css/font-family-pictograph.html

[WebKit Review Bot]

Created attachment 194716 [details]
Archive of layout-test-results from gce-cr-linux-08 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-08  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

[Tomas Popela]

I've test this patch and it fixes all our troubles in Evolution (Even those that https://bugs.webkit.org/show_bug.cgi?id=111714 didn't solve)..

[Gustavo Noronha (kov)]

Comment on attachment 194712 [details]
Patch

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

Makes sense to me.

> Source/WebCore/bindings/gobject/WebKitDOMEventTargetPrivate.h:30
> +WebKitDOMEventTarget* wrapEventTarget(WebCore::EventTarget*);

This doesn't seem to exist after the changes. Leftover?

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1276
>      my $lowerCaseIfaceName = "webkit_dom_" . FixUpDecamelizedName(decamelize($interfaceName));
>      my $parentImplClassName = GetParentImplClassName($interface);
> +    my $baseClass = GetBaseClass($parentImplClassName);

Nit: all the other variables have the 'Name' suffix, so it makes sense to keep the convention here.

[Carlos Garcia Campos]

(In reply to comment #5)
> (From update of attachment 194712 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194712&action=review
> 
> Makes sense to me.

Thanks for the review.

> > Source/WebCore/bindings/gobject/WebKitDOMEventTargetPrivate.h:30
> > +WebKitDOMEventTarget* wrapEventTarget(WebCore::EventTarget*);
> 
> This doesn't seem to exist after the changes. Leftover?

hmm, that's right, EvenTarget is an interface, it shouldn't have a wrapEvenTarget method.

> > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1276
> >      my $lowerCaseIfaceName = "webkit_dom_" . FixUpDecamelizedName(decamelize($interfaceName));
> >      my $parentImplClassName = GetParentImplClassName($interface);
> > +    my $baseClass = GetBaseClass($parentImplClassName);
> 
> Nit: all the other variables have the 'Name' suffix, so it makes sense to keep the convention here.

Sure

[Carlos Garcia Campos]

Committed r147924: <http://trac.webkit.org/changeset/147924>