71334 2011-11-01 15:50:12 -0700 Small fixes for WebVTTParser 2011-11-01 22:48:35 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 annacc annacc darin eric.carlson webkit.review.bot oldest_to_newest 494240 0 annacc 2011-11-01 15:50:12 -0700 hasLongWebVTTIdentifier needs to return true even for WebVTT Identifiers that are not long (exactly 6 chars when they match "WEBVTT"). 494250 1 113246 annacc 2011-11-01 15:57:06 -0700 Created attachment 113246 Patch 494277 2 113246 darin 2011-11-01 16:29:39 -0700 Comment on attachment 113246 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113246&action=review > Source/WebCore/ChangeLog:8 > + No new tests. This is needed to enable other tests, coming soon. OK. I’m going to hold you to that one! > Source/WebCore/html/track/WebVTTParser.cpp:193 > - if (line[position++] != ' ' && line[position++] != '\t') > + if (line[position] != ' ' && line[position] != '\t') What guarantees we are not at the end of the line here? By the way, I think the ++ was OK, but not two different ++. A local variable would have been another way to do it. > Source/WebCore/html/track/WebVTTParser.cpp:201 > - if (line[position++] != ' ' && line[position++] != '\t') > + if (line[position] != ' ' && line[position] != '\t') Same question/comment. 494384 3 113246 annacc 2011-11-01 18:18:54 -0700 Comment on attachment 113246 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113246&action=review >> Source/WebCore/ChangeLog:8 >> + No new tests. This is needed to enable other tests, coming soon. > > OK. I’m going to hold you to that one! Thanks Darin. >> Source/WebCore/html/track/WebVTTParser.cpp:193 >> + if (line[position] != ' ' && line[position] != '\t') > > What guarantees we are not at the end of the line here? > > By the way, I think the ++ was OK, but not two different ++. A local variable would have been another way to do it. Good point, I'll add a check. I suppose it doesn't matter since the next action is to ignore remaining white space, but it would eliminate one extra check. I'll use a local var like you suggest. >> Source/WebCore/html/track/WebVTTParser.cpp:201 >> + if (line[position] != ' ' && line[position] != '\t') > > Same question/comment. Ditto. 494430 4 113276 annacc 2011-11-01 21:42:56 -0700 Created attachment 113276 Patch for landing 494440 5 113276 webkit.review.bot 2011-11-01 22:48:29 -0700 Comment on attachment 113276 Patch for landing Clearing flags on attachment: 113276 Committed r99039: <http://trac.webkit.org/changeset/99039> 494441 6 webkit.review.bot 2011-11-01 22:48:35 -0700 All reviewed patches have been landed. Closing bug. 113246 2011-11-01 15:57:06 -0700 2011-11-01 21:42:52 -0700 Patch bug-71334-20111101155705.patch text/plain 2497 annacc U3VidmVyc2lvbiBSZXZpc2lvbjogOTg5OTgKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCA4NmNjMzkwOWMzZjFmZGJk MzIyOGJhNWFjODBkM2IxNjM2NGY5MjllLi4xY2Y1OGRkOWNlZDkwM2UyNmVjMzZmNzUwYzY3ZjYy MzdjOGZkNDI0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTEtMTEtMDEgIEFubmEg Q2F2ZW5kZXIgIDxhbm5hY2NAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFNtYWxsIGZpeGVzIGZv ciBXZWJWVFRQYXJzZXIuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD03MTMzNAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIE5vIG5ldyB0ZXN0cy4gIFRoaXMgaXMgbmVlZGVkIHRvIGVuYWJsZSBvdGhlciB0ZXN0 cywgY29taW5nIHNvb24uCisKKyAgICAgICAgKiBodG1sL3RyYWNrL1dlYlZUVFBhcnNlci5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpoYXNMb25nV2ViVlRUSWRlbnRpZmllcik6IGNoYW5nZWQgdG8g cmV0dXJuIHRydWUgd2hlbiBoZWFkZXIgaXMKKyAgICAgICAgICAgIGV4YWN0bHkgIldFQlZUVCIK KyAgICAgICAgKFdlYkNvcmU6OldlYlZUVFBhcnNlcjo6Y29sbGVjdFRpbWluZ3NBbmRTZXR0aW5n cyk6ICBmaXggdHlwb3MsIHBvc2l0aW9uIHNob3VsZAorICAgICAgICAgICAgbm90IHByb2dyZXNz IHdoZW4gY2hlY2tpbmcgdGhlIGNoYXJhY3RlciBhZnRlciBhIHRpbWVzdGFtcC4KKwogMjAxMS0x MS0wMSAgRG9taW5pYyBDb29uZXkgIDxkb21pbmljY0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAg ZGlzcGxheTogdGFibGUtY2VsbCBhbmQgYm94LXNpemluZzogYm9yZGVyLWJveCBjYWxjdWxhdGVz IGNvbnRlbnQtYm94IGhlaWdodApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC90cmFj ay9XZWJWVFRQYXJzZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC90cmFjay9XZWJWVFRQYXJz ZXIuY3BwCmluZGV4IGU5ZDAxZDU3ZDRlMzQ0NGIyMzg2ZDhjNjQyYzA5NGE2NGI2NGJkM2UuLmIx MWU2ZmYxZTQ0YzgyZThiYmM3NWZmOTM1NjVhZjFmMzZkMjRhMTggMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL2h0bWwvdHJhY2svV2ViVlRUUGFyc2VyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29y ZS9odG1sL3RyYWNrL1dlYlZUVFBhcnNlci5jcHAKQEAgLTY0LDcgKzY0LDcgQEAgaW5saW5lIGJv b2wgaGFzTG9uZ1dlYlZUVElkZW50aWZpZXIoU3RyaW5nIGxpbmUpCiAgICAgICAgIHJldHVybiBm YWxzZTsKIAogICAgIC8vIG9yIHRoZSBzZXZlbnRoIGNoYXJhY3RlciBpcyBuZWl0aGVyIGEgc3Bh Y2Ugbm9yIGEgdGFiIGNoYXJhY3RlciwgdGhlbiBhYm9ydC4KLSAgICBpZiAobGluZVtmaWxlSWRl bnRpZmVyTGVuZ3RoXSAhPSAnICcgJiYgbGluZVtmaWxlSWRlbnRpZmVyTGVuZ3RoXSAhPSAnXHQn KQorICAgIGlmIChsaW5lLmxlbmd0aCgpID4gZmlsZUlkZW50aWZlckxlbmd0aCAmJiBsaW5lW2Zp bGVJZGVudGlmZXJMZW5ndGhdICE9ICcgJyAmJiBsaW5lW2ZpbGVJZGVudGlmZXJMZW5ndGhdICE9 ICdcdCcpCiAgICAgICAgIHJldHVybiBmYWxzZTsKIAogICAgIHJldHVybiB0cnVlOwpAQCAtMTkw LDcgKzE5MCw3IEBAIFdlYlZUVFBhcnNlcjo6UGFyc2VTdGF0ZSBXZWJWVFRQYXJzZXI6OmNvbGxl Y3RUaW1pbmdzQW5kU2V0dGluZ3MoY29uc3QgU3RyaW5nJiBsCiAgICAgbV9jdXJyZW50U3RhcnRU aW1lID0gY29sbGVjdFRpbWVTdGFtcChsaW5lLCAmcG9zaXRpb24pOwogICAgIGlmIChtX2N1cnJl bnRTdGFydFRpbWUgPT0gbWFsZm9ybWVkVGltZSkKICAgICAgICAgcmV0dXJuIEJhZEN1ZTsKLSAg ICBpZiAobGluZVtwb3NpdGlvbisrXSAhPSAnICcgJiYgbGluZVtwb3NpdGlvbisrXSAhPSAnXHQn KQorICAgIGlmIChsaW5lW3Bvc2l0aW9uXSAhPSAnICcgJiYgbGluZVtwb3NpdGlvbl0gIT0gJ1x0 JykKICAgICAgICAgcmV0dXJuIEJhZEN1ZTsKICAgICBza2lwV2hpdGVTcGFjZShsaW5lLCAmcG9z aXRpb24pOwogCkBAIC0xOTgsNyArMTk4LDcgQEAgV2ViVlRUUGFyc2VyOjpQYXJzZVN0YXRlIFdl YlZUVFBhcnNlcjo6Y29sbGVjdFRpbWluZ3NBbmRTZXR0aW5ncyhjb25zdCBTdHJpbmcmIGwKICAg ICBpZiAobGluZS5maW5kKCItLT4iLCBwb3NpdGlvbikgPT0gbm90Rm91bmQpCiAgICAgICAgIHJl dHVybiBCYWRDdWU7CiAgICAgcG9zaXRpb24gKz0gMzsKLSAgICBpZiAobGluZVtwb3NpdGlvbisr XSAhPSAnICcgJiYgbGluZVtwb3NpdGlvbisrXSAhPSAnXHQnKQorICAgIGlmIChsaW5lW3Bvc2l0 aW9uXSAhPSAnICcgJiYgbGluZVtwb3NpdGlvbl0gIT0gJ1x0JykKICAgICAgICAgcmV0dXJuIEJh ZEN1ZTsKICAgICBza2lwV2hpdGVTcGFjZShsaW5lLCAmcG9zaXRpb24pOwogCg== 113276 2011-11-01 21:42:56 -0700 2011-11-01 22:48:29 -0700 Patch for landing bug-71334-20111101214254.patch text/plain 2657 annacc U3VidmVyc2lvbiBSZXZpc2lvbjogOTkwMzgKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBmNGQzYmZmZTkwMWNhNWVh OTU5Y2JmMTU3NDQ1NTkzZGQ1YjkzZDQwLi5jNWM1NjUyYTFiOGQzOGQ0ZDU0ZjllZmM3N2JmNGQ5 YThkZjAzODBlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTEtMTEtMDEgIEFubmEg Q2F2ZW5kZXIgIDxhbm5hY2NAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFNtYWxsIGZpeGVzIGZv ciBXZWJWVFRQYXJzZXIuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD03MTMzNAorCisgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgorCisgICAg ICAgIE5vIG5ldyB0ZXN0cy4gIFRoaXMgaXMgbmVlZGVkIHRvIGVuYWJsZSBvdGhlciB0ZXN0cywg Y29taW5nIHNvb24uCisKKyAgICAgICAgKiBodG1sL3RyYWNrL1dlYlZUVFBhcnNlci5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpoYXNMb25nV2ViVlRUSWRlbnRpZmllcik6IGNoYW5nZWQgdG8gcmV0 dXJuIHRydWUgd2hlbiBoZWFkZXIgaXMKKyAgICAgICAgICAgIGV4YWN0bHkgIldFQlZUVCIKKyAg ICAgICAgKFdlYkNvcmU6OldlYlZUVFBhcnNlcjo6Y29sbGVjdFRpbWluZ3NBbmRTZXR0aW5ncyk6 IGZpeCB0eXBvcywgcG9zaXRpb24gc2hvdWxkCisgICAgICAgICAgICBvbmx5IHByb2dyZXNzIG9u Y2Ugd2hlbiBjaGVja2luZyB0aGUgY2hhcmFjdGVyIGFmdGVyIGEgdGltZXN0YW1wLgorCiAyMDEx LTExLTAxICBEYXJpbiBBZGxlciAgPGRhcmluQGFwcGxlLmNvbT4KIAogICAgICAgICBDaGFuZ2Ug SFRNTFNlbGVjdEVsZW1lbnQ6OnNldFNlbGVjdGVkSW5kZXggdG8gdXNlIGVudW1zIGluc3RlYWQg b2YgYm9vbHMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvdHJhY2svV2ViVlRUUGFy c2VyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvdHJhY2svV2ViVlRUUGFyc2VyLmNwcAppbmRl eCBlOWQwMWQ1N2Q0ZTM0NDRiMjM4NmQ4YzY0MmMwOTRhNjRiNjRiZDNlLi45MTQ5MTVkZjA4OTU4 MWIyOWY2NzIwNmQ1YTExZWVkZjQwMjAxYzAyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9o dG1sL3RyYWNrL1dlYlZUVFBhcnNlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaHRtbC90cmFj ay9XZWJWVFRQYXJzZXIuY3BwCkBAIC02NCw3ICs2NCw3IEBAIGlubGluZSBib29sIGhhc0xvbmdX ZWJWVFRJZGVudGlmaWVyKFN0cmluZyBsaW5lKQogICAgICAgICByZXR1cm4gZmFsc2U7CiAKICAg ICAvLyBvciB0aGUgc2V2ZW50aCBjaGFyYWN0ZXIgaXMgbmVpdGhlciBhIHNwYWNlIG5vciBhIHRh YiBjaGFyYWN0ZXIsIHRoZW4gYWJvcnQuCi0gICAgaWYgKGxpbmVbZmlsZUlkZW50aWZlckxlbmd0 aF0gIT0gJyAnICYmIGxpbmVbZmlsZUlkZW50aWZlckxlbmd0aF0gIT0gJ1x0JykKKyAgICBpZiAo bGluZS5sZW5ndGgoKSA+IGZpbGVJZGVudGlmZXJMZW5ndGggJiYgbGluZVtmaWxlSWRlbnRpZmVy TGVuZ3RoXSAhPSAnICcgJiYgbGluZVtmaWxlSWRlbnRpZmVyTGVuZ3RoXSAhPSAnXHQnKQogICAg ICAgICByZXR1cm4gZmFsc2U7CiAKICAgICByZXR1cm4gdHJ1ZTsKQEAgLTE5MCw3ICsxOTAsMTAg QEAgV2ViVlRUUGFyc2VyOjpQYXJzZVN0YXRlIFdlYlZUVFBhcnNlcjo6Y29sbGVjdFRpbWluZ3NB bmRTZXR0aW5ncyhjb25zdCBTdHJpbmcmIGwKICAgICBtX2N1cnJlbnRTdGFydFRpbWUgPSBjb2xs ZWN0VGltZVN0YW1wKGxpbmUsICZwb3NpdGlvbik7CiAgICAgaWYgKG1fY3VycmVudFN0YXJ0VGlt ZSA9PSBtYWxmb3JtZWRUaW1lKQogICAgICAgICByZXR1cm4gQmFkQ3VlOwotICAgIGlmIChsaW5l W3Bvc2l0aW9uKytdICE9ICcgJyAmJiBsaW5lW3Bvc2l0aW9uKytdICE9ICdcdCcpCisgICAgaWYg KHBvc2l0aW9uID49IGxpbmUubGVuZ3RoKCkpCisgICAgICAgIHJldHVybiBCYWRDdWU7CisgICAg Y2hhciBuZXh0Q2hhciA9IGxpbmVbcG9zaXRpb24rK107CisgICAgaWYgKG5leHRDaGFyICE9ICcg JyAmJiBuZXh0Q2hhciAhPSAnXHQnKQogICAgICAgICByZXR1cm4gQmFkQ3VlOwogICAgIHNraXBX aGl0ZVNwYWNlKGxpbmUsICZwb3NpdGlvbik7CiAKQEAgLTE5OCw3ICsyMDEsMTAgQEAgV2ViVlRU UGFyc2VyOjpQYXJzZVN0YXRlIFdlYlZUVFBhcnNlcjo6Y29sbGVjdFRpbWluZ3NBbmRTZXR0aW5n cyhjb25zdCBTdHJpbmcmIGwKICAgICBpZiAobGluZS5maW5kKCItLT4iLCBwb3NpdGlvbikgPT0g bm90Rm91bmQpCiAgICAgICAgIHJldHVybiBCYWRDdWU7CiAgICAgcG9zaXRpb24gKz0gMzsKLSAg ICBpZiAobGluZVtwb3NpdGlvbisrXSAhPSAnICcgJiYgbGluZVtwb3NpdGlvbisrXSAhPSAnXHQn KQorICAgIGlmIChwb3NpdGlvbiA+PSBsaW5lLmxlbmd0aCgpKQorICAgICAgICByZXR1cm4gQmFk Q3VlOworICAgIG5leHRDaGFyID0gbGluZVtwb3NpdGlvbisrXTsKKyAgICBpZiAobmV4dENoYXIg IT0gJyAnICYmIG5leHRDaGFyICE9ICdcdCcpCiAgICAgICAgIHJldHVybiBCYWRDdWU7CiAgICAg c2tpcFdoaXRlU3BhY2UobGluZSwgJnBvc2l0aW9uKTsKIAo=