Bug 51464

Summary: [Qt] document.getElementById(...) doesn't return the right object in combination with QGraphicsWidget
Product: WebKit Reporter: thomas.senyk
Component: Plug-insAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, commit-queue, noam, qt-info, suresh.voruganti
Priority: P2    
Version: 420+   
Hardware: PC   
OS: Linux   
Bug Depends on: 51330    
Bug Blocks:    
Attachments:
Description Flags
example code
none
Patch
kling: review+, kling: commit-queue-
Patch none

Description thomas.senyk 2010-12-22 05:36:37 PST
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
Comment 1 Noam Rosenthal 2010-12-31 13:39:10 PST
To apply a fix to this upstream, https://bugs.webkit.org/show_bug.cgi?id=51330 has to be fixed/reverted first.
Comment 2 Noam Rosenthal 2011-01-01 15:15:21 PST
Created attachment 77761 [details]
Patch
Comment 3 Suresh Voruganti 2011-01-03 06:38:45 PST
Fix required for Qtwebkit 2.1, top issue for Qt team
Comment 4 Suresh Voruganti 2011-01-03 06:45:21 PST
As per the comments, adding dependency to 51330. pls cherry pick both the fixes for Qtwebkit 2.1 and 2.2
Comment 5 Andreas Kling 2011-01-03 07:33:13 PST
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/
Comment 6 Simon Hausmann 2011-01-03 07:40:07 PST
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.
Comment 7 Andreas Kling 2011-01-03 07:56:02 PST
Comment on attachment 77761 [details]
Patch

I agree with Simon.
Comment 8 Noam Rosenthal 2011-01-03 08:00:13 PST
(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.
Comment 9 Noam Rosenthal 2011-01-03 10:23:17 PST
Created attachment 77819 [details]
Patch
Comment 10 WebKit Commit Bot 2011-01-03 11:02:12 PST
Comment on attachment 77819 [details]
Patch

Clearing flags on attachment: 77819

Committed r74909: <http://trac.webkit.org/changeset/74909>
Comment 11 WebKit Commit Bot 2011-01-03 11:02:19 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Ademar Reis 2011-01-06 11:27:43 PST
(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?
Comment 13 Ademar Reis 2011-01-11 14:13:40 PST
Revision r74909 cherry-picked into qtwebkit-2.1 with commit 56678c1 <http://gitorious.org/webkit/qtwebkit/commit/56678c1>
Comment 14 Noam Rosenthal 2011-01-30 09:18:39 PST
*** Bug 52053 has been marked as a duplicate of this bug. ***