Bug 51464 – [Qt] document.getElementById(...) doesn't return the right object in combination with QGraphicsWidget

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

Created attachment 77207 [details]
example code

document.getElementById(...) doesn't return the right object if the plugin (provided by QWebPluginFactory) behind it is a QGraphicsWidget ... it returns something, but you can't call any property/function


I attached a example where you can see the problem.

The important files are:
testplugin.h   (there you can change the type of TestPlugin by commenting/uncommenting the #define BUGREPORT_USE_QWIDGET
testpluginfectory.cpp   ... in TestPluginFactory::create is a note which is maybe important
test.html      see function testFunc() ... to see the output: right-click on html-side => "inspect" => "console"



Possible setups:
1. the best working setup:
TestPlugin is a QWidget => everything seems to work with and without addToJavaScriptWindowObject (see note in TestPluginFactory::create)

2. the worst setup
TestPlugin is a QGraphicsWidget && addToJavaScriptWindowObject is NOT executed => nothing in test.hmtl=>testFunc() works

3. at least something is working
TestPlugin is a QGraphicsWidget && addToJavaScriptWindowObject is executed => at least: console.debug("foobar:" + foobar.doSomething())     works

[Noam Rosenthal]

To apply a fix to this upstream, https://bugs.webkit.org/show_bug.cgi?id=51330 has to be fixed/reverted first.

[Noam Rosenthal]

Created attachment 77761 [details]
Patch

[Suresh Voruganti]

Fix required for Qtwebkit 2.1, top issue for Qt team

[Suresh Voruganti]

As per the comments, adding dependency to 51330. pls cherry pick both the fixes for Qtwebkit 2.1 and 2.2

[Andreas Kling]

Comment on attachment 77761 [details]
Patch

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

Looks oky-doky to me.

> WebKit/qt/ChangeLog:9
> +        by adding a custom membe to WebCore::Widget. 

Typo, s/membe/member/

[Simon Hausmann]

Comment on attachment 77761 [details]
Patch

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

I think overall the patch looks good, but I suggest the above renamings/fixes before landing.

> WebCore/platform/Widget.h:239
> +    QObject* qtObject() const;
> +    void setQtObject(QObject*);

I think this property should be called bindingsObject() instead of qtObject. Makes IMHO more sense in the context of WebCore::Widget.

> WebCore/platform/Widget.h:287
> +    QObject* m_qtObject;

I suppose for safety purposes this could be a QWeakPointer.

[Andreas Kling]

Comment on attachment 77761 [details]
Patch

I agree with Simon.

[Noam Rosenthal]

(In reply to comment #6)
> (From update of attachment 77761 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77761&action=review
> 
> I think overall the patch looks good, but I suggest the above renamings/fixes before landing.
> 
> > WebCore/platform/Widget.h:239
> > +    QObject* qtObject() const;
> > +    void setQtObject(QObject*);
> 
> I think this property should be called bindingsObject() instead of qtObject. Makes IMHO more sense in the context of WebCore::Widget.
> 
> > WebCore/platform/Widget.h:287
> > +    QObject* m_qtObject;
> 
> I suppose for safety purposes this could be a QWeakPointer.

Will do.

[Noam Rosenthal]

Created attachment 77819 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 77819 [details]
Patch

Clearing flags on attachment: 77819

Committed r74909: <http://trac.webkit.org/changeset/74909>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ademar Reis]

(In reply to comment #1)
> To apply a fix to this upstream, https://bugs.webkit.org/show_bug.cgi?id=51330 has to be fixed/reverted first.

The patch has been commited already, even though bug 51330 is still open. Shouldn't this bug still be open as well?

[Ademar Reis]

Revision r74909 cherry-picked into qtwebkit-2.1 with commit 56678c1 <http://gitorious.org/webkit/qtwebkit/commit/56678c1>

[Noam Rosenthal]

*** Bug 52053 has been marked as a duplicate of this bug. ***