Bug 36187

Created attachment 53872 [details] Patch

Simon, we need this for the release as it will be used a lot by one of our target groups, and we want them to start using the right API and not the old one.

It seems that the class also changed name. I will do an additional patch to make that change.

Comment on attachment 53872 [details] Patch I apparently changed the expected result by changing the PASS: line (after testing), thus I will have to update the expected result for one Qt test.

http://trac.webkit.org/changeset/57924 might have broken Leopard Intel Release (Tests) The following changes are on the blame list: http://trac.webkit.org/changeset/57924 http://trac.webkit.org/changeset/57925

I have new chromium baselines ready to go whenever this relands, FYI.

I wonder if we need a better way to do these "enumerate the global scope" tests. It seems like a poor design that every port needs different results based on which features they happened to have enabled.

(In reply to comment #6) > I have new chromium baselines ready to go whenever this relands, FYI. Can you attach these to the bug report; I will reland the patch (with updated results) thursday when I'm back at work.

Created attachment 53965 [details] Patch with updated expected results

Comment on attachment 53965 [details] Patch with updated expected results Landing former patch (+ updated expected results) via commit-queue. Updated expected results are marked as unreviewed in the ChangeLog as these do not need review.

Comment on attachment 53872 [details] Patch Cleared Simon Fraser's review+ from obsolete attachment 53872 [details] so that this bug does not appear in http://webkit.org/pending-commit.

(In reply to comment #11) > (From update of attachment 53872 [details]) > Cleared Simon Fraser's review+ from obsolete attachment 53872 [details] so that this bug > does not appear in http://webkit.org/pending-commit. I wanted the last patch to this bug to be committed using the commit-queue, but it doesn't show up in pending-commit. I wanted to commit it this way to make sure that I do not break the build, but if that is not posisble I will have to either request re-review or commit it myself :-(

(In reply to comment #12) > (In reply to comment #11) > > (From update of attachment 53872 [details] [details]) > > Cleared Simon Fraser's review+ from obsolete attachment 53872 [details] [details] so that this bug > > does not appear in http://webkit.org/pending-commit. > > I wanted the last patch to this bug to be committed using the commit-queue, but > it doesn't show up in pending-commit. I wanted to commit it this way to make > sure that I do not break the build, but if that is not posisble I will have to > either request re-review or commit it myself :-( Ah pending-commit is r+'ed patches. Any way to see which patches are on the commit-queue?

No easy way to list the patches, but you can list the bugs: https://bugs.webkit.org/buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=commit-queue%2B&order=Last+Changed

(In reply to comment #14) > No easy way to list the patches, but you can list the bugs: > https://bugs.webkit.org/buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=commit-queue%2B&order=Last+Changed Thanks, I will bookmark that. Anyway, it would be nice to be able to see what number a patch is in the queue of the commit-queue, maybe as a future enhancement.

Comment on attachment 53965 [details] Patch with updated expected results Clearing flags on attachment: 53965 Committed r58085: <http://trac.webkit.org/changeset/58085>

All reviewed patches have been landed. Closing bug.

Created attachment 54083 [details] Rename Media interface to StyleMedia Second part of the change.

The spec also requires a rename of the interface itself.

Attachment 54083 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/1802055

Attachment 54083 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/css/Media.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebCore/css/Media.h:27: #ifndef header guard has wrong style, please use: Media_h [build/header_guard] [5] Total errors found: 2 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.

(In reply to comment #21) > Attachment 54083 [details] did not pass style-queue: > > Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 > WebCore/css/Media.cpp:28: Found other header before a header this file > implements. Should be: config.h, primary header, blank line, and then > alphabetically sorted. [build/include_order] [4] > WebCore/css/Media.h:27: #ifndef header guard has wrong style, please use: > Media_h [build/header_guard] [5] > Total errors found: 2 in 18 files > > > If any of these errors are false positives, please file a bug against > check-webkit-style. Funny, it complains about style issues in Media.cpp etc which actually doesn't exists anymore as the patch renames it to StyleMedia.cpp

(In reply to comment #20) > Attachment 54083 [details] did not build on qt: > Build output: http://webkit-commit-queue.appspot.com/results/1802055 Compiles on Qt here; it might need a clean build I'm afraid. Ossy, could you test this? I hope I won't run into such problems on all platforms.

(In reply to comment #23) > (In reply to comment #20) > > Attachment 54083 [details] [details] did not build on qt: > > Build output: http://webkit-commit-queue.appspot.com/results/1802055 > > Compiles on Qt here; it might need a clean build I'm afraid. Ossy, could you > test this? I hope I won't run into such problems on all platforms. Strange it cannot find StyleMedia.idl, it is like the patch doesn't apply correctly. The patch says: diff --git a/WebCore/css/Media.idl b/WebCore/css/StyleMedia.idl similarity index 93% rename from WebCore/css/Media.idl rename to WebCore/css/StyleMedia.idl index 1bf5900..7be35cc 100644 --- a/WebCore/css/Media.idl +++ b/WebCore/css/StyleMedia.idl

Created attachment 54087 [details] Patch

Attachment 54087 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/css/Media.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebCore/css/Media.h:27: #ifndef header guard has wrong style, please use: Media_h [build/header_guard] [5] Total errors found: 2 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.

Attachment 54087 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/1807043

webkit-patch apply-attachment 54087 [details] M LayoutTests/ChangeLog M LayoutTests/fast/dom/prototype-inheritance-2-expected.txt M LayoutTests/platform/win/fast/dom/prototype-inheritance-2-expected.txt M WebCore/Android.derived.jscbindings.mk M WebCore/Android.derived.v8bindings.mk M WebCore/ChangeLog M WebCore/GNUmakefile.am M WebCore/WebCore.gypi M WebCore/WebCore.pri M WebCore/WebCore.pro M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/WebCore.xcodeproj/project.pbxproj D WebCore/css/Media.cpp D WebCore/css/Media.h D WebCore/css/Media.idl M WebCore/page/DOMWindow.cpp M WebCore/page/DOMWindow.h M WebCore/page/DOMWindow.idl It deletes the files to be renamed and it doesnt create any new files.

Can anyone help me creating a valid diff which includes renaming from a git repo? If not, I cannot make use of the bots and will have to land this manually at some point. If I do that I will do so in the morning so that any breakage should be fixed before California working hours.

Attachment 54087 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/1880006

Attachment 54087 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/1834043

Comment on attachment 54087 [details] Patch Patch looks good, but the diff for renaming the files is not understood by our tools. This patch can only be landed manually or has to be regenerated from an svn repo.

Comment on attachment 54087 [details] Patch As Kenneth promised to land it manually correctly.

(In reply to comment #32) > (From update of attachment 54087 [details]) > Patch looks good, but the diff for renaming the files is not understood by our > tools. This patch can only be landed manually or has to be regenerated from an > svn repo. (In reply to comment #33) > (From update of attachment 54087 [details]) > As Kenneth promised to land it manually correctly. Why was this landed (in r58168) when nearly every try bot turned up red on the final patch?!

Patch landed in r58168. <http://trac.webkit.org/changeset/58168> Follow-up build and test results fixes (with more likely on the way): <http://trac.webkit.org/changeset/58169> <http://trac.webkit.org/changeset/58170> <http://trac.webkit.org/changeset/58171> <http://trac.webkit.org/changeset/58172> <http://trac.webkit.org/changeset/58173> <http://trac.webkit.org/changeset/58174>

(In reply to comment #35) > Patch landed in r58168. <http://trac.webkit.org/changeset/58168> > > Follow-up build and test results fixes (with more likely on the way): > <http://trac.webkit.org/changeset/58169> > <http://trac.webkit.org/changeset/58170> > <http://trac.webkit.org/changeset/58171> > <http://trac.webkit.org/changeset/58172> > <http://trac.webkit.org/changeset/58173> > <http://trac.webkit.org/changeset/58174> <http://trac.webkit.org/changeset/58175>

Comment on attachment 54087 [details] Patch Landed in 58168 with lots of trouble. Thanks for everyone helping me get the bots green as fast as possible.

> Why was this landed (in r58168) when nearly every try bot turned up red on the > final patch?! My mistake. As it was a rename it wouldn't apply the patch to run on the EWS bots (see comment 28), thus I decided to land it in the morning and fix the issues as fast as possible, though I hadn't imagined that there would be so many. Sorry for any trouble, and thank you very much for the help.

(In reply to comment #38) > Sorry for any trouble, and thank you very much for the help. I will move to Mac OS X for development so that I can test better patches as this one in the future.

(In reply to comment #29) > Can anyone help me creating a valid diff which includes renaming from a git > repo? > > If not, I cannot make use of the bots and will have to land this manually at > some point. If I do that I will do so in the morning so that any breakage > should be fixed before California working hours. The svn-create-patch script does this. However, git patches with renames don't work yet. See Bug 32834.

Please cherry-pick this one Simon.

(In reply to comment #41) > Please cherry-pick this one Simon. I mean Simon Hausmann. Sorry about the confusion Simon Fraser :-)

Revision r58085 cherry-picked into qtwebkit-2.0 with commit 5b263f2040886fc71100cb507d2e80f963021496 Revision r58168 cherry-picked into qtwebkit-2.0 with commit 35a9c2373f8260aab20d3f1b1fcb862fb0f35694