20561 2008-08-28 12:03:11 -0700 "isFoo()" methods on CSSValue and subclasses should all be const 2008-08-28 15:29:39 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED P2 Normal --- 1 simon.fraser simon.fraser darin hyatt sam oldest_to_newest 89661 0 simon.fraser 2008-08-28 12:03:11 -0700 CSSValue implements: virtual bool isValueList() { return false; } and CSSValueList has both: public: virtual bool isValueList() const { return true; } and private: virtual bool isValueList() { return true; } Much hilarity ensues. 89663 1 darin 2008-08-28 12:11:46 -0700 Making all be const or all be non-const would be equally good -- obviously a mix is bad! I don't think it's all that useful to have const member functions on objects that are in tree-like structures like the DOM classes. 89679 2 23059 simon.fraser 2008-08-28 14:20:35 -0700 Created attachment 23059 Patch, changelog 89680 3 23059 hyatt 2008-08-28 14:26:53 -0700 Comment on attachment 23059 Patch, changelog I don't think the derived class isValueList() should be private. Put it where the old non-const one was instead? r=me 89681 4 hyatt 2008-08-28 14:27:07 -0700 Unless it compiles, in which case never mind. :) 89683 5 simon.fraser 2008-08-28 14:29:51 -0700 Committed r35978 M WebCore/ChangeLog M WebCore/css/FontValue.h M WebCore/css/CSSValueList.h M WebCore/css/CSSValue.h M WebCore/css/CSSTimingFunctionValue.h r35978 = 28bad1a29d4699bd6fdaf11e87b2361af4dd856a (trunk) 89687 6 darin 2008-08-28 14:45:16 -0700 (In reply to comment #3) > I don't think the derived class isValueList() should be private. Put it where > the old non-const one was instead? There's a reason to make it private. If you already have a CSSValueList and you call isValueList() it'd be great if you got a compile time error, so then you'd realize it was unnecessary to check. That's why I usually like to have these functions private once they're in a class where it's a "dumb thing to ask". Overriding the base class still works fine, because overriding ignores public vs. private. 89690 7 hyatt 2008-08-28 15:29:39 -0700 Yeah I figured that out after commenting in the bug. Thanks for confirming my suspicions. 23059 2008-08-28 14:20:35 -0700 2008-08-28 14:26:53 -0700 Patch, changelog const_methods.txt text/plain 3227 simon.fraser ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NzM1ZTJlNy4uYjY2Y2EwYSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMSBAQAorMjAwOC0wOC0yOCAgU2ltb24gRnJhc2Vy ICA8c2ltb24uZnJhc2VyQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBNYWtlIGFsbCB0aGUgJ2lzRm9vKCknIG1ldGhvZHMgb24gQ1NT VmFsdWUgY29uc3QsCisgICAgICAgIGFuZCBmaXggdGhlIHN1YmNsYXNzZXMuCisgICAgICAgIAor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjA1NjEKKyAg ICAgICAgCisgICAgICAgICogY3NzL0NTU1RpbWluZ0Z1bmN0aW9uVmFsdWUuaDoKKyAgICAgICAg KiBjc3MvQ1NTVmFsdWUuaDoKKyAgICAgICAgKFdlYkNvcmU6OkNTU1ZhbHVlOjppc0ZvbnRWYWx1 ZSk6CisgICAgICAgIChXZWJDb3JlOjpDU1NWYWx1ZTo6aXNJbWFnZUdlbmVyYXRvclZhbHVlKToK KyAgICAgICAgKFdlYkNvcmU6OkNTU1ZhbHVlOjppc0ltYWdlVmFsdWUpOgorICAgICAgICAoV2Vi Q29yZTo6Q1NTVmFsdWU6OmlzSW1wbGljaXRJbml0aWFsVmFsdWUpOgorICAgICAgICAqIGNzcy9D U1NWYWx1ZUxpc3QuaDoKKyAgICAgICAgKiBjc3MvRm9udFZhbHVlLmg6CisKIDIwMDgtMDgtMjgg IEJyYWQgR2FyY2lhICA8YmdhcmNpYUBnb29nbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5 IERhbiBCZXJuc3RlaW4uCmRpZmYgLS1naXQgYS9XZWJDb3JlL2Nzcy9DU1NUaW1pbmdGdW5jdGlv blZhbHVlLmggYi9XZWJDb3JlL2Nzcy9DU1NUaW1pbmdGdW5jdGlvblZhbHVlLmgKaW5kZXggZmZh NDU1Yy4uZTQ2NTk5MyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9jc3MvQ1NTVGltaW5nRnVuY3Rpb25W YWx1ZS5oCisrKyBiL1dlYkNvcmUvY3NzL0NTU1RpbWluZ0Z1bmN0aW9uVmFsdWUuaApAQCAtNTQs NyArNTQsNyBAQCBwcml2YXRlOgogICAgIHsKICAgICB9CiAKLSAgICB2aXJ0dWFsIGJvb2wgaXNU aW1pbmdGdW5jdGlvblZhbHVlKCkgeyByZXR1cm4gdHJ1ZTsgfQorICAgIHZpcnR1YWwgYm9vbCBp c1RpbWluZ0Z1bmN0aW9uVmFsdWUoKSBjb25zdCB7IHJldHVybiB0cnVlOyB9CiAgICAgCiAgICAg ZG91YmxlIG1feDE7CiAgICAgZG91YmxlIG1feTE7CmRpZmYgLS1naXQgYS9XZWJDb3JlL2Nzcy9D U1NWYWx1ZS5oIGIvV2ViQ29yZS9jc3MvQ1NTVmFsdWUuaAppbmRleCBmZmRmNDJmLi4yZjc4NDcw IDEwMDY0NAotLS0gYS9XZWJDb3JlL2Nzcy9DU1NWYWx1ZS5oCisrKyBiL1dlYkNvcmUvY3NzL0NT U1ZhbHVlLmgKQEAgLTQ3LDEzICs0NywxMyBAQCBwdWJsaWM6CiAgICAgdmlydHVhbCBTdHJpbmcg Y3NzVGV4dCgpIGNvbnN0ID0gMDsKICAgICB2b2lkIHNldENzc1RleHQoY29uc3QgU3RyaW5nJiwg RXhjZXB0aW9uQ29kZSYpIHsgfSAvLyBGSVhNRTogTm90IGltcGxlbWVudGVkLgogCi0gICAgdmly dHVhbCBib29sIGlzRm9udFZhbHVlKCkgeyByZXR1cm4gZmFsc2U7IH0KKyAgICB2aXJ0dWFsIGJv b2wgaXNGb250VmFsdWUoKSBjb25zdCB7IHJldHVybiBmYWxzZTsgfQogICAgIHZpcnR1YWwgYm9v bCBpc0ltYWdlR2VuZXJhdG9yVmFsdWUoKSBjb25zdCB7IHJldHVybiBmYWxzZTsgfQogICAgIHZp cnR1YWwgYm9vbCBpc0ltYWdlVmFsdWUoKSBjb25zdCB7IHJldHVybiBmYWxzZTsgfQogICAgIHZp cnR1YWwgYm9vbCBpc0ltcGxpY2l0SW5pdGlhbFZhbHVlKCkgY29uc3QgeyByZXR1cm4gZmFsc2U7 IH0KICAgICB2aXJ0dWFsIGJvb2wgaXNQcmltaXRpdmVWYWx1ZSgpIGNvbnN0IHsgcmV0dXJuIGZh bHNlOyB9Ci0gICAgdmlydHVhbCBib29sIGlzVGltaW5nRnVuY3Rpb25WYWx1ZSgpIHsgcmV0dXJu IGZhbHNlOyB9Ci0gICAgdmlydHVhbCBib29sIGlzVmFsdWVMaXN0KCkgeyByZXR1cm4gZmFsc2U7 IH0KKyAgICB2aXJ0dWFsIGJvb2wgaXNUaW1pbmdGdW5jdGlvblZhbHVlKCkgY29uc3QgeyByZXR1 cm4gZmFsc2U7IH0KKyAgICB2aXJ0dWFsIGJvb2wgaXNWYWx1ZUxpc3QoKSBjb25zdCB7IHJldHVy biBmYWxzZTsgfQogCiAjaWYgRU5BQkxFKFNWRykKICAgICB2aXJ0dWFsIGJvb2wgaXNTVkdDb2xv cigpIGNvbnN0IHsgcmV0dXJuIGZhbHNlOyB9CmRpZmYgLS1naXQgYS9XZWJDb3JlL2Nzcy9DU1NW YWx1ZUxpc3QuaCBiL1dlYkNvcmUvY3NzL0NTU1ZhbHVlTGlzdC5oCmluZGV4IDNlY2NiZjkuLjAw NmI5ZGIgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvY3NzL0NTU1ZhbHVlTGlzdC5oCisrKyBiL1dlYkNv cmUvY3NzL0NTU1ZhbHVlTGlzdC5oCkBAIC00Niw4ICs0Niw2IEBAIHB1YmxpYzoKIAogICAgIHZp cnR1YWwgfkNTU1ZhbHVlTGlzdCgpOwogCi0gICAgdmlydHVhbCBib29sIGlzVmFsdWVMaXN0KCkg Y29uc3QgeyByZXR1cm4gdHJ1ZTsgfQotCiAgICAgc2l6ZV90IGxlbmd0aCgpIGNvbnN0IHsgcmV0 dXJuIG1fdmFsdWVzLnNpemUoKTsgfQogICAgIENTU1ZhbHVlKiBpdGVtKHVuc2lnbmVkKTsKICAg ICBDU1NWYWx1ZSogaXRlbVdpdGhvdXRCb3VuZHNDaGVjayh1bnNpZ25lZCBpbmRleCkgeyByZXR1 cm4gbV92YWx1ZXNbaW5kZXhdLmdldCgpOyB9CkBAIC02NCw3ICs2Miw3IEBAIHByb3RlY3RlZDoK ICAgICBDU1NWYWx1ZUxpc3QoQ1NTUGFyc2VyVmFsdWVMaXN0Kik7CiAKIHByaXZhdGU6Ci0gICAg dmlydHVhbCBib29sIGlzVmFsdWVMaXN0KCkgeyByZXR1cm4gdHJ1ZTsgfQorICAgIHZpcnR1YWwg Ym9vbCBpc1ZhbHVlTGlzdCgpIGNvbnN0IHsgcmV0dXJuIHRydWU7IH0KIAogICAgIHZpcnR1YWwg dW5zaWduZWQgc2hvcnQgY3NzVmFsdWVUeXBlKCkgY29uc3Q7CiAKZGlmZiAtLWdpdCBhL1dlYkNv cmUvY3NzL0ZvbnRWYWx1ZS5oIGIvV2ViQ29yZS9jc3MvRm9udFZhbHVlLmgKaW5kZXggMWEwYTM4 Yy4uNWM3NDA2YyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9jc3MvRm9udFZhbHVlLmgKKysrIGIvV2Vi Q29yZS9jc3MvRm9udFZhbHVlLmgKQEAgLTQ5LDcgKzQ5LDcgQEAgcHVibGljOgogcHJpdmF0ZToK ICAgICBGb250VmFsdWUoKSB7IH0KIAotICAgIHZpcnR1YWwgYm9vbCBpc0ZvbnRWYWx1ZSgpIHsg cmV0dXJuIHRydWU7IH0KKyAgICB2aXJ0dWFsIGJvb2wgaXNGb250VmFsdWUoKSBjb25zdCB7IHJl dHVybiB0cnVlOyB9CiB9OwogCiB9IC8vIG5hbWVzcGFjZQo=