148849 2015-09-04 16:37:20 -0700 [iOS] Need a test for bug #145539: Uploading an animated GIF from the photo library uploads a JPEG 2015-09-04 17:59:46 -0700 1 1 1 Unclassified WebKit Tools / Tests Safari 9 Unspecified iOS 9.0 RESOLVED FIXED P2 Normal --- 1 jhoneycutt jhoneycutt aestes dbates joepeck oldest_to_newest 1123728 0 jhoneycutt 2015-09-04 16:37:20 -0700 Need a test for bug #145539: Uploading an animated GIF from the photo library uploads a JPEG. 1123740 1 260651 jhoneycutt 2015-09-04 16:52:20 -0700 Created attachment 260651 Patch 1123774 2 260651 dbates 2015-09-04 17:35:34 -0700 Comment on attachment 260651 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=260651&action=review > ManualTests/ios/image-file-uploads-as-original-type.html:9 > + if (!arrayBuffer || arrayBuffer.byteLength < 6) { Instead of hardcoding the length of the GIF header (6), I suggest that we use the expression '"GIF89a".length' so as to make it clear to a reader how we came to require that the file contain at least 6 bytes: if (!arrayBuffer || arrayBuffer.byteLength < "GIF89a".length) { > ManualTests/ios/image-file-uploads-as-original-type.html:15 > + var view = new Uint8Array(arrayBuffer, 0, 3); > + var signature = String.fromCharCode(view[0], view[1], view[2]); > + if (signature !== "GIF") { This is OK as-is. Since we are requiring that the file contain at least six bytes we may want to consider ensuring that signature is equal to the string "GIF89a". If you choose to compare to "GIF89a" then I suggest we define a constant say GIFHeader := "GIF89a" and then write line 9, line 13, and line 15 in terms of it. > ManualTests/ios/image-file-uploads-as-original-type.html:16 > + document.getElementById("console").innerHTML = "TEST FAILED. File was not of correct type."; This is OK as-is. It is sufficient to use either textContent or innerText here since there is no markup in the string. > ManualTests/ios/image-file-uploads-as-original-type.html:19 > + document.getElementById("console").innerHTML = "TEST PASSED"; Ditto. > ManualTests/ios/image-file-uploads-as-original-type.html:41 > + <input type=file onchange="runTest(this)"> The attribute value quoting style for attribute type (no quotes around "file") differs from the quoting style used for other HTML attribute values in this file. I suggest that we add quotes around the attribute value for attribute type. Regardless, we should pick an attribute quote style and stick with it throughout this file for consistency. 1123785 3 jhoneycutt 2015-09-04 17:58:45 -0700 Committed r189410: <http://trac.webkit.org/changeset/189410> 1123786 4 jhoneycutt 2015-09-04 17:59:46 -0700 (In reply to comment #2) Thanks for the review! I made your suggested changes before landing. 260651 2015-09-04 16:52:20 -0700 2015-09-04 17:35:34 -0700 Patch bug-148849-20150904165218.patch text/plain 3112 jhoneycutt U3VidmVyc2lvbiBSZXZpc2lvbjogMTg4MzQ0CmRpZmYgLS1naXQgYS9DaGFuZ2VMb2cgYi9DaGFu Z2VMb2cKaW5kZXggODI4YjRkNzBmMDMwZWMzM2ExOGU1MTJlYjJiNzJjYWJhZGRkZDc0My4uOTkx MGE5MmJjZDVkYjRhZmIyMDZkZjIwNmM2MzY2MzZkZTlhOWE0YyAxMDA2NDQKLS0tIGEvQ2hhbmdl TG9nCisrKyBiL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDE1LTA5LTA0ICBKb24gSG9u ZXljdXR0ICA8amhvbmV5Y3V0dEBhcHBsZS5jb20+CisKKyAgICAgICAgW2lPU10gTmVlZCBhIHRl c3QgZm9yIGJ1ZyAjMTQ1NTM5OiBVcGxvYWRpbmcgYW4gYW5pbWF0ZWQgR0lGIGZyb20gdGhlCisg ICAgICAgIHBob3RvIGxpYnJhcnkgdXBsb2FkcyBhIEpQRUcKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0ODg0OQorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogTWFudWFsVGVzdHMvaW9zL2ltYWdlLWZpbGUt dXBsb2Fkcy1hcy1vcmlnaW5hbC10eXBlLmh0bWw6IEFkZGVkLgorICAgICAgICBPbiB0aGUgTWFj LCBmaWxlIHVwbG9hZCB0ZXN0cyB1c2UgRXZlbnRTZW5kZXI6OmJlZ2luRHJhZ1dpdGhGaWxlcy4g VGhpcworICAgICAgICBmdW5jdGlvbmFsaXR5IGRvZXNuJ3QgZXhpc3Qgb24gaU9TLCBzbyB3ZSds bCBhZGQgYSBtYW51YWwgdGVzdC4KKyAgICAgICAgVGhpcyB0ZXN0cyB0aGF0IHRoZSBmaXJzdCBm ZXcgYnl0ZXMgb2YgdGhlIHNlbGVjdGVkIGZpbGUgYXJlICJHSUYiLgorCiAyMDE1LTA4LTEyICBB bGV4IENocmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CiAKICAgICAgICAgRml4 IERlYnVnIENNYWtlIGJ1aWxkcyBvbiBXaW5kb3dzCmRpZmYgLS1naXQgYS9NYW51YWxUZXN0cy9p b3MvaW1hZ2UtZmlsZS11cGxvYWRzLWFzLW9yaWdpbmFsLXR5cGUuaHRtbCBiL01hbnVhbFRlc3Rz L2lvcy9pbWFnZS1maWxlLXVwbG9hZHMtYXMtb3JpZ2luYWwtdHlwZS5odG1sCm5ldyBmaWxlIG1v ZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAu LjhhODEzZGNjMzBkYjQ5MTc4YTc2YTQ5NTk5NThkYjM0NTFiODdlMTEKLS0tIC9kZXYvbnVsbAor KysgYi9NYW51YWxUZXN0cy9pb3MvaW1hZ2UtZmlsZS11cGxvYWRzLWFzLW9yaWdpbmFsLXR5cGUu aHRtbApAQCAtMCwwICsxLDQ0IEBACis8IURPQ1RZUEUgaHRtbD4KKzxodG1sPgorICAgIDxoZWFk PgorICAgICAgICA8c2NyaXB0PgorICAgICAgICAgICAgZnVuY3Rpb24gcnVuVGVzdChmaWxlQ29u dHJvbCkgeworICAgICAgICAgICAgICAgIHZhciByZWFkZXIgPSBuZXcgRmlsZVJlYWRlcigpOwor ICAgICAgICAgICAgICAgIHJlYWRlci5vbmxvYWQgPSBmdW5jdGlvbihlKSB7CisgICAgICAgICAg ICAgICAgICAgIHZhciBhcnJheUJ1ZmZlciA9IHJlYWRlci5yZXN1bHQ7CisgICAgICAgICAgICAg ICAgICAgIGlmICghYXJyYXlCdWZmZXIgfHwgYXJyYXlCdWZmZXIuYnl0ZUxlbmd0aCA8IDYpIHsK KyAgICAgICAgICAgICAgICAgICAgICAgIGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJjb25zb2xl IikuaW5uZXJIVE1MID0gIlRFU1QgRkFJTEVELiBGYWlsZWQgdG8gZ2V0IGFycmF5IGJ1ZmZlciBm b3IgZmlsZS4iOworICAgICAgICAgICAgICAgICAgICAgICAgcmV0dXJuOworICAgICAgICAgICAg ICAgICAgICB9CisgICAgICAgICAgICAgICAgICAgIHZhciB2aWV3ID0gbmV3IFVpbnQ4QXJyYXko YXJyYXlCdWZmZXIsIDAsIDMpOworICAgICAgICAgICAgICAgICAgICB2YXIgc2lnbmF0dXJlID0g U3RyaW5nLmZyb21DaGFyQ29kZSh2aWV3WzBdLCB2aWV3WzFdLCB2aWV3WzJdKTsKKyAgICAgICAg ICAgICAgICAgICAgaWYgKHNpZ25hdHVyZSAhPT0gIkdJRiIpIHsKKyAgICAgICAgICAgICAgICAg ICAgICAgIGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJjb25zb2xlIikuaW5uZXJIVE1MID0gIlRF U1QgRkFJTEVELiBGaWxlIHdhcyBub3Qgb2YgY29ycmVjdCB0eXBlLiI7CisgICAgICAgICAgICAg ICAgICAgICAgICByZXR1cm47CisgICAgICAgICAgICAgICAgICAgIH0KKyAgICAgICAgICAgICAg ICAgICAgZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoImNvbnNvbGUiKS5pbm5lckhUTUwgPSAiVEVT VCBQQVNTRUQiOworICAgICAgICAgICAgICAgIH0KKyAgICAgICAgICAgICAgICByZWFkZXIucmVh ZEFzQXJyYXlCdWZmZXIoZmlsZUNvbnRyb2wuZmlsZXNbMF0pOworICAgICAgICAgICAgfQorICAg ICAgICA8L3NjcmlwdD4KKyAgICA8L2hlYWQ+CisgICAgPGJvZHk+CisgICAgICAgIDxkaXY+Cisg ICAgICAgICAgICBUaGlzIGlzIGEgdGVzdCBmb3IgQnVnICM8YSBocmVmPSJodHRwOi8vdHJhYy53 ZWJraXQub3JnL2NoYW5nZXNldC8xODUyNDEiPjE4NTI0MTwvYT46IFtpT1NdIFVwbG9hZGluZyBh biBhbmltYXRlZCBHSUYgZnJvbSB0aGUgcGhvdG8gbGlicmFyeSB1cGxvYWRzIGEgSlBFRy4KKyAg ICAgICAgICAgIDxicj48YnI+CisgICAgICAgICAgICBUaGlzIHRlc3Qgc2hvdWxkIGJlIHJ1biBv biBpT1MuIElmIHRoZSB0ZXN0IHBhc3NlcywgVEVTVCBQQVNTRUQgd2lsbCBhcHBlYXIgYmVsb3cu CisgICAgICAgICAgICA8YnI+PGJyPgorICAgICAgICAgICAgPGRpdiBpZD0iY29uc29sZSI+V2Fp dGluZyBvbiB0ZXN0IHRvIGNvbXBsZXRlLi4uPC9kaXY+CisgICAgICAgICAgICA8YnI+PGJyPgor ICAgICAgICA8L2Rpdj4KKyAgICAgICAgPGRpdj4KKyAgICAgICAgICAgIExvbmctcHJlc3Mgb24g dGhpcyBpbWFnZSwgYW5kIHNhdmUgaXQgdG8geW91ciBwaG90byBsaWJyYXJ5OgorICAgICAgICAg ICAgPGltZyBzcmM9Ii4uL3Jlc291cmNlcy9ub24tYW5pbWF0ZWQuZ2lmIj4KKyAgICAgICAgPC9k aXY+CisgICAgICAgIDxicj4KKyAgICAgICAgPGRpdj4KKyAgICAgICAgICAgIFRhcCAiQ2hvb3Nl IEZpbGUsIiBhbmQgc2VsZWN0IHRoZSBpbWFnZSBmcm9tIHlvdXIgcGhvdG8gbGlicmFyeToKKyAg ICAgICAgICAgIDxpbnB1dCB0eXBlPWZpbGUgb25jaGFuZ2U9InJ1blRlc3QodGhpcykiPgorICAg ICAgICA8L2Rpdj4KKyAgICA8L2JvZHk+Cis8L2h0bWw+Cg==