21443 2008-10-07 13:13:44 -0700 Allow HAVE_ACCESSIBILITY to be overridden at build time 2010-06-10 16:17:58 -0700 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) Mac OS X 10.5 NEW P2 Normal --- 1 amanda webkit-unassigned alp darin ddkilzer oldest_to_newest 94420 0 amanda 2008-10-07 13:13:44 -0700 Allow the HAVE_ACCESSIBILITY build setting to be overridden at build time, rather than keying only off of the platform, as a number of other build settings already permit. 94422 1 24155 amanda 2008-10-07 13:14:43 -0700 Created attachment 24155 patch to allow HAVE_ACCESSIBILITY to be overridden at build time. 94424 2 24155 darin 2008-10-07 13:22:01 -0700 Comment on attachment 24155 patch to allow HAVE_ACCESSIBILITY to be overridden at build time. I don't think this is correct. The HAVE_ macros are supposed to represent the presence of a certain underlying capability, not whether we want to take advantage of it or not. If we want ACCESSIBILITY to be something we can turn on and off, then it probably needs to be an ENABLE thing instead. Or this might be yet an another PLATFORM(MAC) and PLATFORM(WIN) snafu of some sort. Also, tabs in ChangeLog. 94428 3 alp 2008-10-07 13:33:32 -0700 As Darin says, accessibility should probably have been an ENABLE, not a HAVE. Would be nice if the patch could address that. Moreover, the #if PLATFORM(MAC) && HAVE(ACCESSIBILITY) change doesn't make much sense. If you don't want accessibility in your port, you probably don't want to compile the Accessibility* classes in the first place. Can you try wrapping the entire classes with HAVE/ENABLE(ACCESSIBILITY) guards instead? Thanks 94431 4 24155 alp 2008-10-07 13:34:17 -0700 Comment on attachment 24155 patch to allow HAVE_ACCESSIBILITY to be overridden at build time. r- based on Darin's comments and mine 94432 5 amanda 2008-10-07 13:39:28 -0700 (In reply to comment #2) That makes sense, though the purpose is to suppress the underlying capability at build time. I don't object to adding an ENABLE flag as well and requiring both to be set, but it seems unnecessarily verbose at first glance (since it would just change every occurrence of "HAVE(ACCESSIBILITY)" to "HAVE(ACCESSIBILITY) && ENABLE(ACCESSIBILITY)". Will fix tabs in changelog--oversight. (In reply to comment #3) > Moreover, the #if PLATFORM(MAC) && HAVE(ACCESSIBILITY) change doesn't make much > sense. If you don't want accessibility in your port, you probably don't want to > compile the Accessibility* classes in the first place. Can you try wrapping the > entire classes with HAVE/ENABLE(ACCESSIBILITY) guards instead? Great suggestion. Will try that and upload a new patch. 98000 6 ddkilzer 2008-11-08 04:40:55 -0800 (In reply to comment #2) > (From update of attachment 24155 [edit]) > I don't think this is correct. > > The HAVE_ macros are supposed to represent the presence of a certain underlying > capability, not whether we want to take advantage of it or not. If we want > ACCESSIBILITY to be something we can turn on and off, then it probably needs to > be an ENABLE thing instead. (In reply to comment #3) > As Darin says, accessibility should probably have been an ENABLE, not a HAVE. > Would be nice if the patch could address that. > > Moreover, the #if PLATFORM(MAC) && HAVE(ACCESSIBILITY) change doesn't make much > sense. If you don't want accessibility in your port, you probably don't want to > compile the Accessibility* classes in the first place. Can you try wrapping the > entire classes with HAVE/ENABLE(ACCESSIBILITY) guards instead? (In reply to comment #5) > (In reply to comment #2) > That makes sense, though the purpose is to suppress the underlying capability > at build time. I don't object to adding an ENABLE flag as well and requiring > both to be set, but it seems unnecessarily verbose at first glance (since it > would just change every occurrence of "HAVE(ACCESSIBILITY)" to > "HAVE(ACCESSIBILITY) && ENABLE(ACCESSIBILITY)". See Bug 21802. I think the underlying assumption is that most (all?) platforms have some kind of an accessibility layer, so changing HAVE(ACCESSIBILITY) to ENABLE(ACCESSIBILITY) is the desired fix. I don't think we want to be sprinkling "#if HAVE(ACCESSIBILITY) && ENABLE(ACCESSIBILITY)" everywhere, which seems redundant. :) 98004 7 ddkilzer 2008-11-08 04:55:51 -0800 See also Bug 22137. 98017 8 darin 2008-11-08 09:14:06 -0800 (In reply to comment #6) > I think the underlying assumption is that most (all?) platforms > have some kind of an accessibility layer, so changing HAVE(ACCESSIBILITY) to > ENABLE(ACCESSIBILITY) is the desired fix. I'd say it differently. I'd say that we expect people to only enable accessibility on platforms that have an accessibility layer, and thus we don't need any indication of the presence or absence of the layer. 24155 2008-10-07 13:14:43 -0700 2010-06-10 16:17:58 -0700 patch to allow HAVE_ACCESSIBILITY to be overridden at build time. awalker-a11y-patch text/plain 2392 amanda SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDM3MzgzKQorKysgSmF2YVNjcmlwdENvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTEgQEAKKzIwMDgtMTAtMDcgIEFtYW5kYSBX YWxrZXIgIDxhbWFuZGFAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisJQWxsb3cgSEFWRV9BQ0NFU1NJQklMSVRZIHRvIGJlIG92ZXJyaWRkZW4g YXQgYnVpbGQgdGltZS4KKworICAgICAgICAqIHd0Zi9QbGF0Zm9ybS5oOgorCiAyMDA4LTEwLTA3 ICBNYXJrIFJvd2UgIDxtcm93ZUBhcHBsZS5jb20+CiAKICAgICAgICAgRml4IHR5cG8gaW4gbWV0 aG9kIG5hbWUuCkluZGV4OiBKYXZhU2NyaXB0Q29yZS93dGYvUGxhdGZvcm0uaAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBKYXZhU2NyaXB0Q29yZS93dGYvUGxhdGZvcm0uaAkocmV2aXNpb24gMzczODMpCisrKyBK YXZhU2NyaXB0Q29yZS93dGYvUGxhdGZvcm0uaAkod29ya2luZyBjb3B5KQpAQCAtMjc1LDkgKzI3 NSwxMSBAQAogI2RlZmluZSBXVEZfVVNFX1BUSFJFQURTIDEKICNlbmRpZgogCisjaWYgIWRlZmlu ZWQoSEFWRV9BQ0NFU1NJQklMSVRZKQogI2lmIFBMQVRGT1JNKE1BQykgfHwgUExBVEZPUk0oV0lO KSB8fCBQTEFURk9STShHVEspCiAjZGVmaW5lIEhBVkVfQUNDRVNTSUJJTElUWSAxCiAjZW5kaWYK KyNlbmRpZgogCiAjaWYgQ09NUElMRVIoR0NDKQogI2RlZmluZSBIQVZFX0NPTVBVVEVEX0dPVE8g MQpJbmRleDogV2ViQ29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9DaGFuZ2VM b2cJKHJldmlzaW9uIDM3MzgzKQorKysgV2ViQ29yZS9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkK QEAgLTEsMyArMSwxNCBAQAorMjAwOC0xMC0wNyAgQW1hbmRhIFdhbGtlciAgPGFtYW5kYUBjaHJv bWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgRG9uJ3QgY2FsbCBtZXRob2RzIHRoYXQgYXJlIGNvbmRpdGlvbmFsaXplZCBvbiBIQVZFKEFD Q0VTU0lCSUxJVFkpCisJaWYgaXQgaGFzIGJlZW4gb3ZlcnJpZGRlbiBvZmYgYXQgYnVpbGQgdGlt ZS4KKworICAgICAgICAqIHBhZ2UvQWNjZXNzaWJpbGl0eVJlbmRlck9iamVjdC5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpBY2Nlc3NpYmlsaXR5UmVuZGVyT2JqZWN0Ojpib3VuZHNGb3JWaXNpYmxl UG9zaXRpb25SYW5nZSk6CisgICAgICAgIChXZWJDb3JlOjpBY2Nlc3NpYmlsaXR5UmVuZGVyT2Jq ZWN0Ojp2aXNpYmxlUG9zaXRpb25Gb3JQb2ludCk6CisKIDIwMDgtMTAtMDcgIEFscCBUb2tlciAg PGFscEBudWFudGkuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IE1hcmsgUm93ZS4KSW5kZXg6 IFdlYkNvcmUvcGFnZS9BY2Nlc3NpYmlsaXR5UmVuZGVyT2JqZWN0LmNwcAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBXZWJDb3JlL3BhZ2UvQWNjZXNzaWJpbGl0eVJlbmRlck9iamVjdC5jcHAJKHJldmlzaW9uIDM3 MzgzKQorKysgV2ViQ29yZS9wYWdlL0FjY2Vzc2liaWxpdHlSZW5kZXJPYmplY3QuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC0xNjE5LDcgKzE2MTksNyBAQCBJbnRSZWN0IEFjY2Vzc2liaWxpdHlSZW5k ZXJPYmplY3Q6OmJvdW5kCiAgICAgICAgICAgICBvdXJyZWN0ID0gYm91bmRpbmdCb3g7CiAgICAg fQogICAgIAotI2lmIFBMQVRGT1JNKE1BQykKKyNpZiBQTEFURk9STShNQUMpICYmIEhBVkUoQUND RVNTSUJJTElUWSkKICAgICByZXR1cm4gbV9yZW5kZXJlci0+ZG9jdW1lbnQoKS0+dmlldygpLT5j b250ZW50c1RvU2NyZWVuKG91cnJlY3QpOwogI2Vsc2UKICAgICByZXR1cm4gb3VycmVjdDsKQEAg LTE2NTIsNyArMTY1Miw3IEBAIFZpc2libGVQb3NpdGlvbiBBY2Nlc3NpYmlsaXR5UmVuZGVyT2Jq ZWMKICAgICBJbnRQb2ludCBwb2ludFJlc3VsdDsKICAgICB3aGlsZSAoMSkgewogICAgICAgICBJ bnRQb2ludCBvdXJwb2ludDsKLSNpZiBQTEFURk9STShNQUMpCisjaWYgUExBVEZPUk0oTUFDKSAm JiBIQVZFKEFDQ0VTU0lCSUxJVFkpCiAgICAgICAgIG91cnBvaW50ID0gZnJhbWVWaWV3LT5zY3Jl ZW5Ub0NvbnRlbnRzKHBvaW50KTsKICNlbHNlCiAgICAgICAgIG91cnBvaW50ID0gcG9pbnQ7Cg==