73466 2011-11-30 10:37:31 -0800 HTMLTrackElement.readyState should return TextTrack "readiness state". 2011-12-01 08:48:14 -0800 1 1 1 Unclassified WebKit Media 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 eric.carlson eric.carlson annacc webkit-bug-importer oldest_to_newest 511872 0 eric.carlson 2011-11-30 10:37:31 -0800 r101213 (72553) moved the text track 'readyState' attribute from TextTrack to HTMLTrackElement. The attribute and constants do belong on the element, but all types of text tracks - in-band, out of band, and those created with addTextTrack() - must maintain a "text track readiness state" as well. Additionally, HTMLTrackElement.readyState is supposed to return the readiness state of the track element's TextTrack: The readyState attribute must return the numeric value corresponding to the text track readiness state of the track element's text track [1] [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#dom-track-readystate 511873 1 webkit-bug-importer 2011-11-30 10:38:06 -0800 <rdar://problem/10504834> 511899 2 117225 eric.carlson 2011-11-30 11:01:01 -0800 Created attachment 117225 Proposed patch 512278 3 117225 darin 2011-11-30 17:23:55 -0800 Comment on attachment 117225 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=117225&action=review This code is OK but I am not happy about having two enums that have to always match. Can we fix that? > Source/WebCore/html/HTMLTrackElement.cpp:223 > + ensureTrack()->setReadinessState(static_cast<TextTrack::ReadinessState>(state)); This cast isn’t safe if we ever change one of the two enums without the other. There’s not even a comment to guarantee we keep them in sync. > Source/WebCore/html/HTMLTrackElement.cpp:230 > + return static_cast<HTMLTrackElement::ReadyState>(ensureTrack()->readinessState()); No need here for HTMLTrackElement:: prefix. This cast isn’t safe if we ever change one of the two enums without the other. There’s not even a comment to guarantee we keep them in sync. > Source/WebCore/html/TextTrack.h:113 > + TextTrack::ReadinessState m_readinessState; There is no need for TextTrack:: prefix here. 512797 4 eric.carlson 2011-12-01 08:47:55 -0800 (In reply to comment #3) > (From update of attachment 117225 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117225&action=review > > This code is OK but I am not happy about having two enums that have to always match. Can we fix that? > > > Source/WebCore/html/HTMLTrackElement.cpp:223 > > + ensureTrack()->setReadinessState(static_cast<TextTrack::ReadinessState>(state)); > > This cast isn’t safe if we ever change one of the two enums without the other. There’s not even a comment to guarantee we keep them in sync. > I couldn't come up with a clean way to have just one enum because the enum in HTMLTrackElement.h is visible to JavaScript, and HTMLTrackElement.h has to include TextTrack.h, but I added COMPILE_ASSERTs for all values so HTMLTrackElement.cpp will fail to compile if the two enums ever get out of sync. > > Source/WebCore/html/HTMLTrackElement.cpp:230 > > + return static_cast<HTMLTrackElement::ReadyState>(ensureTrack()->readinessState()); > > No need here for HTMLTrackElement:: prefix. > Fixed. > > Source/WebCore/html/TextTrack.h:113 > > + TextTrack::ReadinessState m_readinessState; > > There is no need for TextTrack:: prefix here. > Fixed. 512798 5 eric.carlson 2011-12-01 08:48:14 -0800 http://trac.webkit.org/changeset/101673 117225 2011-11-30 11:01:01 -0800 2011-11-30 17:23:55 -0800 Proposed patch patch_ready_state_1.txt text/plain 4904 eric.carlson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwMTUyOSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBACisyMDExLTExLTMwICBFcmljIENh cmxzb24gIDxlcmljLmNhcmxzb25AYXBwbGUuY29tPgorCisgICAgICAgIEhUTUxUcmFja0VsZW1l bnQucmVhZHlTdGF0ZSBzaG91bGQgcmV0dXJuIFRleHRUcmFjayAicmVhZGluZXNzIHN0YXRlIi4K KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTczNDY2CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBodG1sL0hU TUxUcmFja0VsZW1lbnQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SFRNTFRyYWNrRWxlbWVudDo6 SFRNTFRyYWNrRWxlbWVudCk6IERvbid0IGluaXRpYWxpemUgbV9yZWFkeVN0YXRlLgorICAgICAg ICAoV2ViQ29yZTo6SFRNTFRyYWNrRWxlbWVudDo6c2V0UmVhZHlTdGF0ZSk6IFJldHVybiB0aGUg VGV4dFRyYWNrIHN0YXRlLgorICAgICAgICAoV2ViQ29yZTo6SFRNTFRyYWNrRWxlbWVudDo6cmVh ZHlTdGF0ZSk6IFNldCB0aGUgVGV4dFRyYWNrIHN0YXRlLgorICAgICAgICAqIGh0bWwvSFRNTFRy YWNrRWxlbWVudC5oOiBSZW1vdmUgbV9yZWFkeVN0YXRlLgorCisgICAgICAgICogaHRtbC9UZXh0 VHJhY2suY3BwOgorICAgICAgICAoV2ViQ29yZTo6VGV4dFRyYWNrOjpUZXh0VHJhY2spOiBJbml0 aWFsaXplIG1fcmVhZGluZXNzU3RhdGUuCisgICAgICAgICogaHRtbC9UZXh0VHJhY2suaDoKKyAg ICAgICAgKFdlYkNvcmU6OlRleHRUcmFjazo6cmVhZGluZXNzU3RhdGUpOiBOZXcuCisgICAgICAg IChXZWJDb3JlOjpUZXh0VHJhY2s6OnNldFJlYWRpbmVzc1N0YXRlKTogTmV3LgorCiAyMDExLTEx LTMwICBBbnR0aSBLb2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXVzZSBj YWNoZWQgc3R5bGUgZnVsbHkgaWYgdGhlIHBhcmVudCBpbmhlcml0ZWQgc3R5bGVzIGFyZSBlcXVh bCAKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTFRyYWNrRWxlbWVudC5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MVHJhY2tFbGVtZW50LmNwcAkocmV2aXNp b24gMTAxMjYzKQorKysgU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MVHJhY2tFbGVtZW50LmNwcAko d29ya2luZyBjb3B5KQpAQCAtNDMsNyArNDMsNiBAQCB1c2luZyBuYW1lc3BhY2UgSFRNTE5hbWVz OwogCiBpbmxpbmUgSFRNTFRyYWNrRWxlbWVudDo6SFRNTFRyYWNrRWxlbWVudChjb25zdCBRdWFs aWZpZWROYW1lJiB0YWdOYW1lLCBEb2N1bWVudCogZG9jdW1lbnQpCiAgICAgOiBIVE1MRWxlbWVu dCh0YWdOYW1lLCBkb2N1bWVudCkKLSAgICAsIG1fcmVhZHlTdGF0ZShIVE1MVHJhY2tFbGVtZW50 OjpOT05FKQogewogICAgIExPRyhNZWRpYSwgIkhUTUxUcmFja0VsZW1lbnQ6OkhUTUxUcmFja0Vs ZW1lbnQgLSAlcCIsIHRoaXMpOwogICAgIEFTU0VSVChoYXNUYWdOYW1lKHRyYWNrVGFnKSk7CkBA IC0yMjEsMTAgKzIyMCwxNSBAQCB2b2lkIEhUTUxUcmFja0VsZW1lbnQ6OmRpZENvbXBsZXRlTG9h ZChMCiAKIHZvaWQgSFRNTFRyYWNrRWxlbWVudDo6c2V0UmVhZHlTdGF0ZShSZWFkeVN0YXRlIHN0 YXRlKQogewotICAgIG1fcmVhZHlTdGF0ZSA9IHN0YXRlOworICAgIGVuc3VyZVRyYWNrKCktPnNl dFJlYWRpbmVzc1N0YXRlKHN0YXRpY19jYXN0PFRleHRUcmFjazo6UmVhZGluZXNzU3RhdGU+KHN0 YXRlKSk7CiAgICAgaWYgKEhUTUxNZWRpYUVsZW1lbnQqIHBhcmVudCA9IG1lZGlhRWxlbWVudCgp KQogICAgICAgICByZXR1cm4gcGFyZW50LT50ZXh0VHJhY2tSZWFkeVN0YXRlQ2hhbmdlZChtX3Ry YWNrLmdldCgpKTsKIH0KKworSFRNTFRyYWNrRWxlbWVudDo6UmVhZHlTdGF0ZSBIVE1MVHJhY2tF bGVtZW50OjpyZWFkeVN0YXRlKCkgCit7CisgICAgcmV0dXJuIHN0YXRpY19jYXN0PEhUTUxUcmFj a0VsZW1lbnQ6OlJlYWR5U3RhdGU+KGVuc3VyZVRyYWNrKCktPnJlYWRpbmVzc1N0YXRlKCkpOwor fQogICAgIAogdm9pZCBIVE1MVHJhY2tFbGVtZW50Ojp0ZXh0VHJhY2tLaW5kQ2hhbmdlZChUZXh0 VHJhY2sqIHRyYWNrKQogewpJbmRleDogU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MVHJhY2tFbGVt ZW50LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MVHJhY2tFbGVtZW50 LmgJKHJldmlzaW9uIDEwMTI2MykKKysrIFNvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTFRyYWNrRWxl bWVudC5oCSh3b3JraW5nIGNvcHkpCkBAIC0zMCw2ICszMCw3IEBACiAKICNpbmNsdWRlICJIVE1M RWxlbWVudC5oIgogI2luY2x1ZGUgIkxvYWRhYmxlVGV4dFRyYWNrLmgiCisjaW5jbHVkZSAiVGV4 dFRyYWNrLmgiCiAKIG5hbWVzcGFjZSBXZWJDb3JlIHsKIApAQCAtNTMsOSArNTQsMTAgQEAgcHVi bGljOgogCiAgICAgYm9vbCBpc0RlZmF1bHQoKSBjb25zdDsKICAgICB2b2lkIHNldElzRGVmYXVs dChib29sKTsKLSAgICAKKwogICAgIGVudW0gUmVhZHlTdGF0ZSB7IE5PTkUgPSAwLCBMT0FESU5H ID0gMSwgTE9BREVEID0gMiwgVFJBQ0tfRVJST1IgPSAzIH07Ci0gICAgUmVhZHlTdGF0ZSByZWFk eVN0YXRlKCkgY29uc3QgeyByZXR1cm4gbV9yZWFkeVN0YXRlOyB9CisgICAgUmVhZHlTdGF0ZSBy ZWFkeVN0YXRlKCk7CisgICAgdm9pZCBzZXRSZWFkeVN0YXRlKFJlYWR5U3RhdGUpOwogCiAgICAg VGV4dFRyYWNrKiB0cmFjaygpOwogICAgIApAQCAtNjMsOCArNjUsNiBAQCBwdWJsaWM6CiAgICAg dmlydHVhbCBib29sIGNhbkxvYWRVcmwoTG9hZGFibGVUZXh0VHJhY2sqLCBjb25zdCBLVVJMJik7 CiAgICAgdmlydHVhbCB2b2lkIGRpZENvbXBsZXRlTG9hZChMb2FkYWJsZVRleHRUcmFjayosIGJv b2wgLyogbG9hZGluZ0ZhaWxlZCAqLyk7CiAgICAgCi0gICAgdm9pZCBzZXRSZWFkeVN0YXRlKFJl YWR5U3RhdGUpOwotCiBwcml2YXRlOgogICAgIEhUTUxUcmFja0VsZW1lbnQoY29uc3QgUXVhbGlm aWVkTmFtZSYsIERvY3VtZW50Kik7CiAgICAgdmlydHVhbCB+SFRNTFRyYWNrRWxlbWVudCgpOwpA QCAtOTMsNyArOTMsNiBAQCBwcml2YXRlOgogCiAgICAgTG9hZGFibGVUZXh0VHJhY2sqIGVuc3Vy ZVRyYWNrKCk7CiAKLSAgICBIVE1MVHJhY2tFbGVtZW50OjpSZWFkeVN0YXRlIG1fcmVhZHlTdGF0 ZTsKICAgICBSZWZQdHI8TG9hZGFibGVUZXh0VHJhY2s+IG1fdHJhY2s7CiB9OwogCkluZGV4OiBT b3VyY2UvV2ViQ29yZS9odG1sL1RleHRUcmFjay5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dl YkNvcmUvaHRtbC9UZXh0VHJhY2suY3BwCShyZXZpc2lvbiAxMDEyNjMpCisrKyBTb3VyY2UvV2Vi Q29yZS9odG1sL1RleHRUcmFjay5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTc4LDYgKzc4LDcgQEAg VGV4dFRyYWNrOjpUZXh0VHJhY2soU2NyaXB0RXhlY3V0aW9uQ29udAogICAgICwgbV9tb2RlKFRl eHRUcmFjazo6SElEREVOKQogICAgICwgbV9jbGllbnQoY2xpZW50KQogICAgICwgbV90cmFja1R5 cGUodHlwZSkKKyAgICAsIG1fcmVhZGluZXNzU3RhdGUoTm90TG9hZGVkKQogewogICAgIHNldEtp bmQoa2luZCk7CiB9CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9odG1sL1RleHRUcmFjay5oCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2h0bWwvVGV4dFRyYWNrLmgJKHJldmlzaW9uIDEwMTI2 MykKKysrIFNvdXJjZS9XZWJDb3JlL2h0bWwvVGV4dFRyYWNrLmgJKHdvcmtpbmcgY29weSkKQEAg LTc5LDEyICs3OSwxMyBAQCBwdWJsaWM6CiAgICAgTW9kZSBtb2RlKCkgY29uc3QgeyByZXR1cm4g bV9tb2RlOyB9CiAgICAgdm9pZCBzZXRNb2RlKHVuc2lnbmVkIHNob3J0LCBFeGNlcHRpb25Db2Rl Jik7CiAKKyAgICBlbnVtIFJlYWRpbmVzc1N0YXRlIHsgTm90TG9hZGVkID0gMCwgTG9hZGluZyA9 IDEsIExvYWRlZCA9IDIsIEZhaWxlZFRvTG9hZCA9IDMgfTsKKyAgICBSZWFkaW5lc3NTdGF0ZSBy ZWFkaW5lc3NTdGF0ZSgpIGNvbnN0IHsgcmV0dXJuIG1fcmVhZGluZXNzU3RhdGU7IH0KKyAgICB2 b2lkIHNldFJlYWRpbmVzc1N0YXRlKFJlYWRpbmVzc1N0YXRlIHN0YXRlKSB7IG1fcmVhZGluZXNz U3RhdGUgPSBzdGF0ZTsgfQorCiAgICAgVGV4dFRyYWNrQ3VlTGlzdCogY3VlcygpOwogICAgIFRl eHRUcmFja0N1ZUxpc3QqIGFjdGl2ZUN1ZXMoKSBjb25zdDsKIAotICAgIHZvaWQgcmVhZHlTdGF0 ZUNoYW5nZWQoKTsKLSAgICB2b2lkIG1vZGVDaGFuZ2VkKCk7Ci0KICAgICB2aXJ0dWFsIHZvaWQg Y2xlYXJDbGllbnQoKSB7IG1fY2xpZW50ID0gMDsgfQogICAgIFRleHRUcmFja0NsaWVudCogY2xp ZW50KCkgeyByZXR1cm4gbV9jbGllbnQ7IH0KIApAQCAtMTA5LDYgKzExMCw3IEBAIHByaXZhdGU6 CiAgICAgVGV4dFRyYWNrOjpNb2RlIG1fbW9kZTsKICAgICBUZXh0VHJhY2tDbGllbnQqIG1fY2xp ZW50OwogICAgIFRleHRUcmFja1R5cGUgbV90cmFja1R5cGU7CisgICAgVGV4dFRyYWNrOjpSZWFk aW5lc3NTdGF0ZSBtX3JlYWRpbmVzc1N0YXRlOwogfTsKIAogfSAvLyBuYW1lc3BhY2UgV2ViQ29y ZQo=