Bug 113132 - [GTK] DOM objects created wrapping a base class have incorrect GObject type
Summary: [GTK] DOM objects created wrapping a base class have incorrect GObject type
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-23 04:46 PDT by Carlos Garcia Campos
Modified: 2013-04-08 10:19 PDT (History)
15 users (show)

See Also:


Attachments
Patch (92.25 KB, patch)
2013-03-23 11:21 PDT, Carlos Garcia Campos
gns: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-08 for chromium-linux-x86_64 (1014.22 KB, application/zip)
2013-03-23 12:58 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2013-03-23 04:46:51 PDT
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.
Comment 1 Carlos Garcia Campos 2013-03-23 11:21:40 PDT
Created attachment 194712 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-23 12:57:59 PDT
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
Comment 3 WebKit Review Bot 2013-03-23 12:58:03 PDT
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
Comment 4 Tomas Popela 2013-03-26 07:46:39 PDT
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)..
Comment 5 Gustavo Noronha (kov) 2013-04-08 06:46:42 PDT
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.
Comment 6 Carlos Garcia Campos 2013-04-08 09:56:10 PDT
(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
Comment 7 Carlos Garcia Campos 2013-04-08 10:19:22 PDT
Committed r147924: <http://trac.webkit.org/changeset/147924>