28196 2009-08-11 15:52:24 -0700 Chromium: Show a "Playback Disabled" button on media error. 2009-08-13 15:50:15 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P4 Normal --- 1 kylep webkit-unassigned ajwong eric levin oldest_to_newest 139148 0 kylep 2009-08-11 15:52:24 -0700 Load and paint a mediaPlayDisabled image in RenderThemeChromiumSkia::paintMediaPlayButton() to display when the mediaElement cannot load. 139149 1 34608 kylep 2009-08-11 15:54:51 -0700 Created attachment 34608 Fix Loads and paints a disabled playback image. 139159 2 34608 levin 2009-08-11 16:25:08 -0700 Comment on attachment 34608 Fix The diff here is odd. It should have WebCore/ prefixed before each file. I suspect that you are working from a chromium directory and that's why you have this problem. (I don't do that so I don't know the fix.) At worst fix it by hand to aid in landing your patch. > Index: ChangeLog > +2009-08-11 Kyle Prete <kylep@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + You need the bug title and a link to the bug here. (see other changelog entries.) prepare-ChangeLog -bug # will give the you the right format. > + Use a disabled play button when the media file cannot be played. > + > + No new tests. (OOPS!) This shouldn't be here. Either add a test or indicate if existing tests cover this. > Index: rendering/RenderThemeChromiumSkia.cpp > static Image* mediaPlay = Image::loadPlatformResource("mediaPlay").releaseRef(); > static Image* mediaPause = Image::loadPlatformResource("mediaPause").releaseRef(); > + static Image* mediaPlayDisabled = Image::loadPlatformResource("mediaPlayDisabled").releaseRef(); > > + if (mediaElement->networkState() == HTMLMediaElement::NETWORK_NO_SOURCE) > + return paintMediaButtonInternal(paintInfo.context, rect, mediaPlayDisabled); This needs a 4 space indent. 139170 3 34615 kylep 2009-08-11 16:57:25 -0700 Created attachment 34615 Fix Thanks for the review. Your comments should be covered by this revision. 139567 4 34615 eric 2009-08-12 14:57:59 -0700 Comment on attachment 34615 Fix Rejecting patch 34615 from commit-queue. This patch will require manual commit. Patch https://bugs.webkit.org/attachment.cgi?id=34615 from bug 28196 failed to download and apply. 139606 5 34615 eric 2009-08-12 15:33:46 -0700 Comment on attachment 34615 Fix Not sure why this was rejected from the queue. 139630 6 34615 eric 2009-08-12 16:08:28 -0700 Comment on attachment 34615 Fix Rejecting patch 34615 from commit-queue. This patch will require manual commit. Patch https://bugs.webkit.org/attachment.cgi?id=34615 from bug 28196 failed to download and apply. 139631 7 eric 2009-08-12 16:09:47 -0700 Oh, this patch was made from within WebCore instead of the top level directory. that's why it's failing: patching file WebCore/ChangeLog patch: **** malformed patch at line 20: patch -p0 "WebCore/ChangeLog" returned 2. Pass --force to ignore patch failures. 139633 8 eric 2009-08-12 16:10:44 -0700 It seems that we could teach svn-apply to handle such patches, especially since the full path is actually in the patch. But maybe this patch is truly malformed? 139679 9 kylep 2009-08-12 17:39:26 -0700 Is it? This is only my second time submitting a patch to Webkit. If I've made a mistake, please let me know so I can fix it. 139712 10 levin 2009-08-12 20:41:27 -0700 Kyle, I think you fixed this issue :"It should have WebCore/ prefixed before each file." by hand, which is ok but you need to fix every file name, Note that there is --- ChangeLog (revision 47069) +++ ChangeLog (working copy) in the patch, for example. 139913 11 34756 kylep 2009-08-13 10:17:40 -0700 Created attachment 34756 Fix Whoops. Ok, added more prefixes - this should work now. 139914 12 34756 eric 2009-08-13 10:20:42 -0700 Comment on attachment 34756 Fix This change is wrong now too. The patch should only have the directories under WebKit in it. svn-create-patch (which you should be using) will do this for you. bugzilla-tool post-diff 28196 will also do this for you. 139941 13 levin 2009-08-13 11:19:52 -0700 He has an odd enlistment. (enlisted in WebCore and JSC etc separately), so the svn-create-patch thing gives the wrong result. (It leaves out WebCore/) Kyle, the top level dir WebKit/shouldn't be in the path but you did fix all the file names this time :) Sorry if I miscommunicated this. 139957 14 ajwong 2009-08-13 11:52:58 -0700 I think this particular structure is going to end up being quite common for people who hack the webkit checkout that's embedded in the chromium source tree. :( Interesitng note, if you specify the paths explicitly like "svn-create-patch WebCore", you will get diff output but the paths will end up rooted off the someplace weird (like the root of the chromium checkout). I'm willing to manually land this if you guys are okay with that. 139998 15 eric 2009-08-13 13:20:04 -0700 Seems like this hybrid checkout is not well supported. Someone needs to file bugs and fix the tools if such a hybrid is desired. 140039 16 34756 levin 2009-08-13 14:33:18 -0700 Comment on attachment 34756 Fix The change is fine. The patch format needs massaging his ajwong has offered to do. 140082 17 ajwong 2009-08-13 15:50:15 -0700 Committed as r47247. 34608 2009-08-11 15:54:51 -0700 2009-08-11 16:57:25 -0700 Fix patch.txt text/plain 1405 kylep SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDQ3MDY5 KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMDktMDgt MTEgIEt5bGUgUHJldGUgIDxreWxlcEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVXNlIGEgZGlzYWJsZWQgcGxheSBidXR0b24g d2hlbiB0aGUgbWVkaWEgZmlsZSBjYW5ub3QgYmUgcGxheWVkLgorCisgICAgICAgIE5vIG5ldyB0 ZXN0cy4gKE9PUFMhKQorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclRoZW1lQ2hyb21pdW1T a2lhLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlclRoZW1lQ2hyb21pdW1Ta2lhOjpwYWlu dE1lZGlhUGxheUJ1dHRvbik6CisKIDIwMDktMDgtMTEgIE1heGltZSBTaW1vbiAgPHNpbW9uLm1h eGltZUBnbWFpbC5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlkZWwuCkluZGV4 OiByZW5kZXJpbmcvUmVuZGVyVGhlbWVDaHJvbWl1bVNraWEuY3BwCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHJl bmRlcmluZy9SZW5kZXJUaGVtZUNocm9taXVtU2tpYS5jcHAJKHJldmlzaW9uIDQ3MDQxKQorKysg cmVuZGVyaW5nL1JlbmRlclRoZW1lQ2hyb21pdW1Ta2lhLmNwcAkod29ya2luZyBjb3B5KQpAQCAt NjEwLDcgKzYxMCwxMSBAQAogCiAgICAgc3RhdGljIEltYWdlKiBtZWRpYVBsYXkgPSBJbWFnZTo6 bG9hZFBsYXRmb3JtUmVzb3VyY2UoIm1lZGlhUGxheSIpLnJlbGVhc2VSZWYoKTsKICAgICBzdGF0 aWMgSW1hZ2UqIG1lZGlhUGF1c2UgPSBJbWFnZTo6bG9hZFBsYXRmb3JtUmVzb3VyY2UoIm1lZGlh UGF1c2UiKS5yZWxlYXNlUmVmKCk7CisgICAgc3RhdGljIEltYWdlKiBtZWRpYVBsYXlEaXNhYmxl ZCA9IEltYWdlOjpsb2FkUGxhdGZvcm1SZXNvdXJjZSgibWVkaWFQbGF5RGlzYWJsZWQiKS5yZWxl YXNlUmVmKCk7CiAKKyAgICBpZiAobWVkaWFFbGVtZW50LT5uZXR3b3JrU3RhdGUoKSA9PSBIVE1M TWVkaWFFbGVtZW50OjpORVRXT1JLX05PX1NPVVJDRSkKKyAgICAgIHJldHVybiBwYWludE1lZGlh QnV0dG9uSW50ZXJuYWwocGFpbnRJbmZvLmNvbnRleHQsIHJlY3QsIG1lZGlhUGxheURpc2FibGVk KTsKKwogICAgIHJldHVybiBwYWludE1lZGlhQnV0dG9uSW50ZXJuYWwocGFpbnRJbmZvLmNvbnRl eHQsIHJlY3QsIG1lZGlhRWxlbWVudC0+cGF1c2VkKCkgPyBtZWRpYVBsYXkgOiBtZWRpYVBhdXNl KTsKICNlbHNlCiAgICAgVU5VU0VEX1BBUkFNKG9iamVjdCk7Cg== 34615 2009-08-11 16:57:25 -0700 2009-08-13 10:17:40 -0700 Fix patch.txt text/plain 1599 kylep SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIENoYW5nZUxvZwkocmV2aXNp b24gNDcwNjkpCisrKyBDaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxNCBAQAor MjAwOS0wOC0xMSAgS3lsZSBQcmV0ZSAgPGt5bGVwQGNocm9taXVtLm9yZz4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDaHJvbWl1bTogU2hvdyBhICJQ bGF5YmFjayBEaXNhYmxlZCIgYnV0dG9uIG9uIG1lZGlhIGVycm9yLgorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjgxOTYKKyAgICAgICAgVXNlIGEgZGlz YWJsZWQgcGxheSBidXR0b24gd2hlbiB0aGUgbWVkaWEgZmlsZSBjYW5ub3QgYmUgcGxheWVkLgor CisgICAgICAgIE5vIG5ldyB0ZXN0cyBuZWVkZWQuIENvdmVyZWQgYnkgTGF5b3V0VGVzdHMvbWVk aWEvdmlkZW8tZW1wdHktc291cmNlLmh0bWwKKworICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJU aGVtZUNocm9taXVtU2tpYS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZUNocm9t aXVtU2tpYTo6cGFpbnRNZWRpYVBsYXlCdXR0b24pOgorCiAyMDA5LTA4LTExICBNYXhpbWUgU2lt b24gIDxzaW1vbi5tYXhpbWVAZ21haWwuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEVyaWMg U2VpZGVsLgpJbmRleDogV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVDaHJvbWl1bVNraWEu Y3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIHJlbmRlcmluZy9SZW5kZXJUaGVtZUNocm9taXVtU2tpYS5jcHAJ KHJldmlzaW9uIDQ3MDQxKQorKysgcmVuZGVyaW5nL1JlbmRlclRoZW1lQ2hyb21pdW1Ta2lhLmNw cAkod29ya2luZyBjb3B5KQpAQCAtNjEwLDcgKzYxMCwxMSBAQAogCiAgICAgc3RhdGljIEltYWdl KiBtZWRpYVBsYXkgPSBJbWFnZTo6bG9hZFBsYXRmb3JtUmVzb3VyY2UoIm1lZGlhUGxheSIpLnJl bGVhc2VSZWYoKTsKICAgICBzdGF0aWMgSW1hZ2UqIG1lZGlhUGF1c2UgPSBJbWFnZTo6bG9hZFBs YXRmb3JtUmVzb3VyY2UoIm1lZGlhUGF1c2UiKS5yZWxlYXNlUmVmKCk7CisgICAgc3RhdGljIElt YWdlKiBtZWRpYVBsYXlEaXNhYmxlZCA9IEltYWdlOjpsb2FkUGxhdGZvcm1SZXNvdXJjZSgibWVk aWFQbGF5RGlzYWJsZWQiKS5yZWxlYXNlUmVmKCk7CiAKKyAgICBpZiAobWVkaWFFbGVtZW50LT5u ZXR3b3JrU3RhdGUoKSA9PSBIVE1MTWVkaWFFbGVtZW50OjpORVRXT1JLX05PX1NPVVJDRSkKKyAg ICAgICAgcmV0dXJuIHBhaW50TWVkaWFCdXR0b25JbnRlcm5hbChwYWludEluZm8uY29udGV4dCwg cmVjdCwgbWVkaWFQbGF5RGlzYWJsZWQpOworCiAgICAgcmV0dXJuIHBhaW50TWVkaWFCdXR0b25J bnRlcm5hbChwYWludEluZm8uY29udGV4dCwgcmVjdCwgbWVkaWFFbGVtZW50LT5wYXVzZWQoKSA/ IG1lZGlhUGxheSA6IG1lZGlhUGF1c2UpOwogI2Vsc2UKICAgICBVTlVTRURfUEFSQU0ob2JqZWN0 KTsK 34756 2009-08-13 10:17:40 -0700 2009-08-13 14:33:18 -0700 Fix patch.txt text/plain 1673 kylep SW5kZXg6IFdlYmtpdC9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJraXQvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDQ3MDY5KQorKysgV2Via2l0L1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMDktMDgtMTEgIEt5bGUgUHJl dGUgIDxreWxlcEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgQ2hyb21pdW06IFNob3cgYSAiUGxheWJhY2sgRGlzYWJsZWQiIGJ1 dHRvbiBvbiBtZWRpYSBlcnJvci4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTI4MTk2CisgICAgICAgIFVzZSBhIGRpc2FibGVkIHBsYXkgYnV0dG9uIHdo ZW4gdGhlIG1lZGlhIGZpbGUgY2Fubm90IGJlIHBsYXllZC4KKworICAgICAgICBObyBuZXcgdGVz dHMgbmVlZGVkLiBDb3ZlcmVkIGJ5IExheW91dFRlc3RzL21lZGlhL3ZpZGVvLWVtcHR5LXNvdXJj ZS5odG1sCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVyVGhlbWVDaHJvbWl1bVNraWEuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyVGhlbWVDaHJvbWl1bVNraWE6OnBhaW50TWVkaWFQ bGF5QnV0dG9uKToKKwogMjAwOS0wOC0xMSAgTWF4aW1lIFNpbW9uICA8c2ltb24ubWF4aW1lQGdt YWlsLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBFcmljIFNlaWRlbC4KSW5kZXg6IFdlYmtp dC9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZUNocm9taXVtU2tpYS5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gV2Via2l0L1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRoZW1lQ2hyb21pdW1Ta2lhLmNw cAkocmV2aXNpb24gNDcwNDEpCisrKyBXZWJraXQvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhl bWVDaHJvbWl1bVNraWEuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC02MTAsNyArNjEwLDExIEBACiAK ICAgICBzdGF0aWMgSW1hZ2UqIG1lZGlhUGxheSA9IEltYWdlOjpsb2FkUGxhdGZvcm1SZXNvdXJj ZSgibWVkaWFQbGF5IikucmVsZWFzZVJlZigpOwogICAgIHN0YXRpYyBJbWFnZSogbWVkaWFQYXVz ZSA9IEltYWdlOjpsb2FkUGxhdGZvcm1SZXNvdXJjZSgibWVkaWFQYXVzZSIpLnJlbGVhc2VSZWYo KTsKKyAgICBzdGF0aWMgSW1hZ2UqIG1lZGlhUGxheURpc2FibGVkID0gSW1hZ2U6OmxvYWRQbGF0 Zm9ybVJlc291cmNlKCJtZWRpYVBsYXlEaXNhYmxlZCIpLnJlbGVhc2VSZWYoKTsKIAorICAgIGlm IChtZWRpYUVsZW1lbnQtPm5ldHdvcmtTdGF0ZSgpID09IEhUTUxNZWRpYUVsZW1lbnQ6Ok5FVFdP UktfTk9fU09VUkNFKQorICAgICAgICByZXR1cm4gcGFpbnRNZWRpYUJ1dHRvbkludGVybmFsKHBh aW50SW5mby5jb250ZXh0LCByZWN0LCBtZWRpYVBsYXlEaXNhYmxlZCk7CisKICAgICByZXR1cm4g cGFpbnRNZWRpYUJ1dHRvbkludGVybmFsKHBhaW50SW5mby5jb250ZXh0LCByZWN0LCBtZWRpYUVs ZW1lbnQtPnBhdXNlZCgpID8gbWVkaWFQbGF5IDogbWVkaWFQYXVzZSk7CiAjZWxzZQogICAgIFVO VVNFRF9QQVJBTShvYmplY3QpOwo=