Bug 48035 – WebWindowFeatures has a faulty constructor for WebCore::WindowFeatures

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[usaini]

In WebWindowFeatures.h, in the constructor of WebWindowFeatures that takes in a WebCore::WindowFeatures, we set the xSet/ySet/widthSet/heightSet variables to the same as the values of the passed in object. However, we do not set the corresponding x,y,width, and height variables, and hence if one of those "Set" boolean variables are true, the corresponding underlying variables will not be initialized. This leads to broken code, especially since I know when an underlying popup is created, this implicit constructor is called to transform objects of type WebCore::WindowFeatures into a WebKit::WebWindowFeature.

[usaini]

Created attachment 71380 [details]
Fix for implicit constructor that translates between WebKit::WebWindowFeature and WebCore::WindowFeature

[usaini]

Created attachment 71464 [details]
patch created via svn-create-patch.

I couldn't run the diff from the root directory strangely.
Also, couldn't find where or if the prepare-ChangeLog script sent the file to disk as I couldn't find it. There were no obvious errors there.

[usaini]

Created attachment 71470 [details]
Review for patch to WebWindowFeatures.h that fixes a constructor call to keep WebWindowFeatures always in a valid state.

Updated the patch to contain the ChangeLog after getting that to work properly.

[Csaba Osztrogonác]

Comment on attachment 71470 [details]
Review for patch to WebWindowFeatures.h that fixes a constructor call to keep WebWindowFeatures always in a valid state.

Please add a changelog entry with WebKitTools/Scripts/prepare-changelog,
and then add a comment to the changelog why this change is needed.

Additionally run svn-create-patch from the root of the WebKit directory
and not from WebKit/chromium. It will make EWS bots happier. :)

[usaini]

Created attachment 71770 [details]
patch created via svn-create-patch.

Sorry, had uploaded the wrong file.

[Antonio Gomes]

Comment on attachment 71770 [details]
patch created via svn-create-patch.

Looks sane to me.

[usaini]

Hi Csaba,

Could you look at this for commit? (Either thumbs up or down?)

Thanks,
Udam

[WebKit Commit Bot]

Comment on attachment 71770 [details]
patch created via svn-create-patch.

Rejecting patch 71770 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', 71770]" exit_code: 2
Last 500 characters of output:
 or --strip option?
The text leading up to this was:
--------------------------
|Index: public/WebWindowFeatures.h
|===================================================================
|--- public/WebWindowFeatures.h	(revision 69808)
|+++ public/WebWindowFeatures.h	(working copy)
--------------------------
No file to patch.  Skipping patch.
1 out of 1 hunk ignored

Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Antonio Gomes', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/4755049

[Antonio Gomes]

> The text leading up to this was:
> --------------------------
> |Index: public/WebWindowFeatures.h
> |===================================================================
> |--- public/WebWindowFeatures.h    (revision 69808)
> |+++ public/WebWindowFeatures.h    (working copy)

please submit patch generated against WebKit trunk.

[usaini]

Created attachment 72251 [details]
patch run on trunk.

I believe this should be right directory on the WebKit trunk.

[usaini]

Hi Csaba,

Could you look at this for commit again?

Thanks,
Udam

[WebKit Commit Bot]

Comment on attachment 72251 [details]
patch run on trunk.

Clearing flags on attachment: 72251

Committed r71391: <http://trac.webkit.org/changeset/71391>

[Csaba Osztrogonác]

Close, because the patch was landed.