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.
Created attachment 71380 [details] Fix for implicit constructor that translates between WebKit::WebWindowFeature and WebCore::WindowFeature
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.
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.
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. :)
Created attachment 71770 [details] patch created via svn-create-patch. Sorry, had uploaded the wrong file.
Comment on attachment 71770 [details] patch created via svn-create-patch. Looks sane to me.
Hi Csaba, Could you look at this for commit? (Either thumbs up or down?) Thanks, Udam
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
> 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.
Created attachment 72251 [details] patch run on trunk. I believe this should be right directory on the WebKit trunk.
Hi Csaba, Could you look at this for commit again? Thanks, Udam
Comment on attachment 72251 [details] patch run on trunk. Clearing flags on attachment: 72251 Committed r71391: <http://trac.webkit.org/changeset/71391>
Close, because the patch was landed.