74443 2011-12-13 13:15:51 -0800 Media url with fragment may not load 2011-12-14 11:01:16 -0800 1 1 1 Unclassified WebKit Media 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 eric.carlson eric.carlson webkit-bug-importer webkit.review.bot oldest_to_newest 520324 0 eric.carlson 2011-12-13 13:15:51 -0800 When MediaPlayer::load is not passed the MIME type for a url, it uses the extension to looks one up in the type cache. The code that extracts the extension searches for a '.' from the end of the url, without consideration for whether the url has a fragment or query. 520325 1 webkit-bug-importer 2011-12-13 13:16:32 -0800 <rdar://problem/10574697> 520372 2 119084 eric.carlson 2011-12-13 14:23:55 -0800 Created attachment 119084 Proposed patch. 520375 3 webkit.review.bot 2011-12-13 14:30:22 -0800 Attachment 119084 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/medi..." exit_code: 1 Source/WebCore/platform/graphics/MediaPlayer.h:211: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style. 520420 4 119084 darin 2011-12-13 15:34:43 -0800 Comment on attachment 119084 Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=119084&action=review > Source/WebCore/platform/graphics/MediaPlayer.cpp:341 > + if (protocolIs(urlString, "data")) Would be better to call url.protocolIsData() since we have the URL. > Source/WebCore/platform/graphics/MediaPlayer.cpp:347 > - size_t pos = url.reverseFind('.'); > + String lastPathComponent = url.lastPathComponent(); > + size_t pos = lastPathComponent.reverseFind('.'); > if (pos != notFound) { > - String extension = url.substring(pos + 1); > + String extension = lastPathComponent.substring(pos + 1); Is this the only place in our code where we get the extension from a URL? If not, maybe they can share more code. > Source/WebCore/platform/graphics/MediaPlayer.h:37 > +#include "KURL.h" This include is not needed. All the header needs is a forward declaration of the KURL class. >> Source/WebCore/platform/graphics/MediaPlayer.h:211 >> - bool load(const String& url, const ContentType&); >> + bool load(const KURL& url, const ContentType&); > > The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5] I agree with the style bot. 521085 5 eric.carlson 2011-12-14 11:01:16 -0800 http://trac.webkit.org/changeset/102800 119084 2011-12-13 14:23:55 -0800 2011-12-13 15:34:43 -0800 Proposed patch. patch_3.txt text/plain 6262 eric.carlson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwMjcwNCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIxIEBACisyMDExLTEyLTEzICBFcmljIENh cmxzb24gIDxlcmljLmNhcmxzb25AYXBwbGUuY29tPgorCisgICAgICAgIE1lZGlhIHVybCB3aXRo IGZyYWdtZW50IG1heSBub3QgbG9hZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9NzQ0NDMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBUZXN0OiBtZWRpYS9tZWRpYS1leHRlbnNpb24td2l0aC1mcmFnbWVudC5o dG1sCisKKyAgICAgICAgKiBodG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwOgorICAgICAgICAoV2Vi Q29yZTo6SFRNTE1lZGlhRWxlbWVudDo6bG9hZFJlc291cmNlKTogUGFzcyB0aGUgS1VSTCB0byBN ZWRpYVBsYXllciwgbGV0IGl0IGV4dHJhY3QgYQorICAgICAgICAgICAgU3RyaW5nIHdoZW4gaXQg bmVlZHMgaXQuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9NZWRpYVBsYXllci5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpNZWRpYVBsYXllcjo6bG9hZCk6IFRha2UgYSBLVVJMLCBub3Qg YSBTdHJpbmcuIExvb2sgZm9yIHRoZSBmaWxlIGV4dGVuc2lvbiBpbiB0aGUgCisgICAgICAgICAg ICBsYXN0IHBhdGggY29tcG9uZW50IHNvIHdlIGRvbid0IGV4YW1pbmUgZnJhZ21lbnRzIGFuZC9v ciBxdWVyaWVzLgorICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL01lZGlhUGxheWVyLmg6CisK IDIwMTEtMTItMTMgIFhpbmduYW4gV2FuZyAgPHhpbmduYW4ud2FuZ0BpbnRlbC5jb20+CiAKICAg ICAgICAgSW1wbGVtZW50IGEgZnVuY3Rpb24gb2YgdmVjdG9yIG11bHRpcGx5IHdpdGggU1NFMiBv cHRpbWl6YXRpb24gaW4gVmVjdG9yTWF0aC5jcHAuCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9odG1s L0hUTUxNZWRpYUVsZW1lbnQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2h0bWwv SFRNTE1lZGlhRWxlbWVudC5jcHAJKHJldmlzaW9uIDEwMTY2NikKKysrIFNvdXJjZS9XZWJDb3Jl L2h0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTg4OCw3ICs4ODgs NyBAQCB2b2lkIEhUTUxNZWRpYUVsZW1lbnQ6OmxvYWRSZXNvdXJjZShjb25zCiAgICAgICAgIG1f bXV0ZWQgPSB0cnVlOwogICAgIHVwZGF0ZVZvbHVtZSgpOwogCi0gICAgaWYgKCFtX3BsYXllci0+ bG9hZCh1cmwuc3RyaW5nKCksIGNvbnRlbnRUeXBlKSkKKyAgICBpZiAoIW1fcGxheWVyLT5sb2Fk KHVybCwgY29udGVudFR5cGUpKQogICAgICAgICBtZWRpYUxvYWRpbmdGYWlsZWQoTWVkaWFQbGF5 ZXI6OkZvcm1hdEVycm9yKTsKIAogICAgIC8vIElmIHRoZXJlIGlzIG5vIHBvc3RlciB0byBkaXNw bGF5LCBhbGxvdyB0aGUgbWVkaWEgZW5naW5lIHRvIHJlbmRlciB2aWRlbyBmcmFtZXMgYXMgc29v biBhcwpJbmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvTWVkaWFQbGF5ZXIu Y3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL01lZGlh UGxheWVyLmNwcAkocmV2aXNpb24gMTAxNjY2KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v Z3JhcGhpY3MvTWVkaWFQbGF5ZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zMzAsMTkgKzMzMCwy MSBAQCBNZWRpYVBsYXllcjo6fk1lZGlhUGxheWVyKCkKICAgICBtX21lZGlhUGxheWVyQ2xpZW50 ID0gMDsKIH0KIAotYm9vbCBNZWRpYVBsYXllcjo6bG9hZChjb25zdCBTdHJpbmcmIHVybCwgY29u c3QgQ29udGVudFR5cGUmIGNvbnRlbnRUeXBlKQorYm9vbCBNZWRpYVBsYXllcjo6bG9hZChjb25z dCBLVVJMJiB1cmwsIGNvbnN0IENvbnRlbnRUeXBlJiBjb250ZW50VHlwZSkKIHsKICAgICBTdHJp bmcgdHlwZSA9IGNvbnRlbnRUeXBlLnR5cGUoKS5sb3dlcigpOwogICAgIFN0cmluZyB0eXBlQ29k ZWNzID0gY29udGVudFR5cGUucGFyYW1ldGVyKGNvZGVjcygpKTsKKyAgICBTdHJpbmcgdXJsU3Ry aW5nID0gdXJsLnN0cmluZygpOwogCiAgICAgLy8gSWYgdGhlIE1JTUUgdHlwZSBpcyBtaXNzaW5n IG9yIGlzIG5vdCBtZWFuaW5nZnVsLCB0cnkgdG8gZmlndXJlIGl0IG91dCBmcm9tIHRoZSBVUkwu CiAgICAgaWYgKHR5cGUuaXNFbXB0eSgpIHx8IHR5cGUgPT0gYXBwbGljYXRpb25PY3RldFN0cmVh bSgpIHx8IHR5cGUgPT0gdGV4dFBsYWluKCkpIHsKLSAgICAgICAgaWYgKHByb3RvY29sSXModXJs LCAiZGF0YSIpKQotICAgICAgICAgICAgdHlwZSA9IG1pbWVUeXBlRnJvbURhdGFVUkwodXJsKTsK KyAgICAgICAgaWYgKHByb3RvY29sSXModXJsU3RyaW5nLCAiZGF0YSIpKQorICAgICAgICAgICAg dHlwZSA9IG1pbWVUeXBlRnJvbURhdGFVUkwodXJsU3RyaW5nKTsKICAgICAgICAgZWxzZSB7Ci0g ICAgICAgICAgICBzaXplX3QgcG9zID0gdXJsLnJldmVyc2VGaW5kKCcuJyk7CisgICAgICAgICAg ICBTdHJpbmcgbGFzdFBhdGhDb21wb25lbnQgPSB1cmwubGFzdFBhdGhDb21wb25lbnQoKTsKKyAg ICAgICAgICAgIHNpemVfdCBwb3MgPSBsYXN0UGF0aENvbXBvbmVudC5yZXZlcnNlRmluZCgnLicp OwogICAgICAgICAgICAgaWYgKHBvcyAhPSBub3RGb3VuZCkgewotICAgICAgICAgICAgICAgIFN0 cmluZyBleHRlbnNpb24gPSB1cmwuc3Vic3RyaW5nKHBvcyArIDEpOworICAgICAgICAgICAgICAg IFN0cmluZyBleHRlbnNpb24gPSBsYXN0UGF0aENvbXBvbmVudC5zdWJzdHJpbmcocG9zICsgMSk7 CiAgICAgICAgICAgICAgICAgU3RyaW5nIG1lZGlhVHlwZSA9IE1JTUVUeXBlUmVnaXN0cnk6Omdl dE1lZGlhTUlNRVR5cGVGb3JFeHRlbnNpb24oZXh0ZW5zaW9uKTsKICAgICAgICAgICAgICAgICBp ZiAoIW1lZGlhVHlwZS5pc0VtcHR5KCkpCiAgICAgICAgICAgICAgICAgICAgIHR5cGUgPSBtZWRp YVR5cGU7CkBAIC0zNTAsNyArMzUyLDcgQEAgYm9vbCBNZWRpYVBsYXllcjo6bG9hZChjb25zdCBT dHJpbmcmIHVybAogICAgICAgICB9CiAgICAgfQogCi0gICAgbV91cmwgPSB1cmw7CisgICAgbV91 cmwgPSB1cmxTdHJpbmc7CiAgICAgbV9jb250ZW50TUlNRVR5cGUgPSB0eXBlOwogICAgIG1fY29u dGVudFR5cGVDb2RlY3MgPSB0eXBlQ29kZWNzOwogICAgIGxvYWRXaXRoTmV4dE1lZGlhRW5naW5l KDApOwpJbmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvTWVkaWFQbGF5ZXIu aAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9NZWRpYVBs YXllci5oCShyZXZpc2lvbiAxMDE2NjYpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9NZWRpYVBsYXllci5oCSh3b3JraW5nIGNvcHkpCkBAIC0zNCw2ICszNCw3IEBACiAKICNp bmNsdWRlICJEb2N1bWVudC5oIgogI2luY2x1ZGUgIkludFJlY3QuaCIKKyNpbmNsdWRlICJLVVJM LmgiCiAjaW5jbHVkZSA8d3RmL0ZvcndhcmQuaD4KICNpbmNsdWRlIDx3dGYvSGFzaFNldC5oPgog I2luY2x1ZGUgPHd0Zi9Pd25QdHIuaD4KQEAgLTIwNyw3ICsyMDgsNyBAQCBwdWJsaWM6CiAgICAg SW50U2l6ZSBzaXplKCkgY29uc3QgeyByZXR1cm4gbV9zaXplOyB9CiAgICAgdm9pZCBzZXRTaXpl KGNvbnN0IEludFNpemUmIHNpemUpOwogCi0gICAgYm9vbCBsb2FkKGNvbnN0IFN0cmluZyYgdXJs LCBjb25zdCBDb250ZW50VHlwZSYpOworICAgIGJvb2wgbG9hZChjb25zdCBLVVJMJiB1cmwsIGNv bnN0IENvbnRlbnRUeXBlJik7CiAgICAgdm9pZCBjYW5jZWxMb2FkKCk7CiAKICAgICBib29sIHZp c2libGUoKSBjb25zdDsKSW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwMjcwNCkKKysrIExheW91dFRlc3Rz L0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDExLTEyLTEzICBF cmljIENhcmxzb24gIDxlcmljLmNhcmxzb25AYXBwbGUuY29tPgorCisgICAgICAgIE1lZGlhIHVy bCB3aXRoIGZyYWdtZW50IG1heSBub3QgbG9hZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzQ0NDMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICAqIG1lZGlhL21lZGlhLWV4dGVuc2lvbi13aXRoLWZyYWdtZW50 LWV4cGVjdGVkLnR4dDogQWRkZWQuCisgICAgICAgICogbWVkaWEvbWVkaWEtZXh0ZW5zaW9uLXdp dGgtZnJhZ21lbnQuaHRtbDogQWRkZWQuCisKIDIwMTEtMTItMTMgIFZzZXZvbG9kIFZsYXNvdiAg PHZzZXZpa0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgV2ViIEluc3BlY3RvcjogTmV0d29yayBp dGVtIHZpZXcgZG9lcyBub3QgY29ycmVjdGx5IGRlY29kZSAiKyIgaW4gcmVxdWVzdCBwYXJhbWV0 ZXJzLgpJbmRleDogTGF5b3V0VGVzdHMvbWVkaWEvbWVkaWEtZXh0ZW5zaW9uLXdpdGgtZnJhZ21l bnQtZXhwZWN0ZWQudHh0Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL21lZGlhL21lZGlhLWV4 dGVuc2lvbi13aXRoLWZyYWdtZW50LWV4cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKKysrIExheW91 dFRlc3RzL21lZGlhL21lZGlhLWV4dGVuc2lvbi13aXRoLWZyYWdtZW50LWV4cGVjdGVkLnR4dAko cmV2aXNpb24gMCkKQEAgLTAsMCArMSw1IEBACitFVkVOVChsb2FkZWRkYXRhKQorTWVkaWFQbGF5 ZXIgZXh0cmFjdGVkIGV4dGVuc2lvbiBmcm9tIHNyYyB1cmwuIE9LCisKK0VORCBPRiBURVNUCisK SW5kZXg6IExheW91dFRlc3RzL21lZGlhL21lZGlhLWV4dGVuc2lvbi13aXRoLWZyYWdtZW50Lmh0 bWwKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvbWVkaWEvbWVkaWEtZXh0ZW5zaW9uLXdpdGgt ZnJhZ21lbnQuaHRtbAkocmV2aXNpb24gMCkKKysrIExheW91dFRlc3RzL21lZGlhL21lZGlhLWV4 dGVuc2lvbi13aXRoLWZyYWdtZW50Lmh0bWwJKHJldmlzaW9uIDApCkBAIC0wLDAgKzEsMzAgQEAK KzwhRE9DVFlQRSBodG1sPgorPGh0bWw+CisgICAgPGhlYWQ+CisgICAgICAgIDxzY3JpcHQgc3Jj PW1lZGlhLWZpbGUuanM+PC9zY3JpcHQ+CisgICAgICAgIDxzY3JpcHQgc3JjPXZpZGVvLXRlc3Qu anM+PC9zY3JpcHQ+CisgICAgICAgIDxzY3JpcHQ+CisgICAgICAgICAgICBmdW5jdGlvbiBmaW5p c2goKQorICAgICAgICAgICAgeworICAgICAgICAgICAgICAgIGxvZ1Jlc3VsdChldmVudC50eXBl ID09ICJsb2FkZWRkYXRhIiwgIk1lZGlhUGxheWVyIGV4dHJhY3RlZCBleHRlbnNpb24gZnJvbSBz cmMgdXJsLiIpOworICAgICAgICAgICAgICAgIGNvbnNvbGVXcml0ZSgiIik7CisgICAgICAgICAg ICAgICAgZW5kVGVzdCgpOworICAgICAgICAgICAgfQorCisKKyAgICAgICAgICAgIGZ1bmN0aW9u IHN0YXJ0KCkKKyAgICAgICAgICAgIHsKKyAgICAgICAgICAgICAgICBmaW5kTWVkaWFFbGVtZW50 KCk7CisgICAgICAgICAgICAgICAgd2FpdEZvckV2ZW50QW5kRW5kKCdsb2FkZWRkYXRhJywgZmlu aXNoKTsKKyAgICAgICAgICAgICAgICB3YWl0Rm9yRXZlbnQoJ2Vycm9yJywgZmluaXNoKTsKKyAg ICAgICAgICAgICAgICB2aWRlby5zcmMgPSBmaW5kTWVkaWFGaWxlKCJ2aWRlbyIsICJjb250ZW50 L3Rlc3QiKSArICIjMDEuMDAueHl6IjsKKyAgICAgICAgICAgIH0KKyAgICAgICAgPC9zY3JpcHQ+ CisKKyAgICA8L2hlYWQ+CisgICAgPGJvZHkgb25sb2FkPSJzdGFydCgpIj4KKworICAgICAgICA8 dmlkZW8gY29udHJvbHMgPjwvdmlkZW8+CisKKyAgICA8L2JvZHk+Cis8L2h0bWw+Cg==