37134 2010-04-05 20:16:32 -0700 Remove unused DIRECTIONAL_PAD_NAVIGATION 2010-04-06 07:33:56 -0700 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) PC All RESOLVED FIXED P2 Normal --- 1 tonikitoo tonikitoo caryclark eric joseph.ligman laszlo.gombos oldest_to_newest 208872 0 tonikitoo 2010-04-05 20:16:32 -0700 bug 23145 was filed intending to add a DIRECTIONAL_PAD_NAVIGATION build flag to WebCore (see attached patch in that bug), but according to https://bugs.webkit.org/show_bug.cgi?id=23145#c3 this is no longer needed once android does not implement ΅directional navigation" in engine side, nor seems to plan so. Bug was then RESOLVED as INVALID. In the meanwhile, bug 23163 landed a patch that relies on the DIRECTIONAL_PAD_NAVIGATION flag, and it is not dead code. I propose to remove any refenrece to DIRECTIONAL_PAD_NAVIGATION from trunk ps: we have "Spatial Navigation" available in trunk (see bug 18862 and dependency list). 208873 1 52601 tonikitoo 2010-04-05 20:23:17 -0700 Created attachment 52601 (committed in r57148) patch 208874 2 tonikitoo 2010-04-05 20:26:22 -0700 s/it is not dead code/it is NOW dead code/ 208876 3 laszlo.gombos 2010-04-05 20:39:37 -0700 Bug 18862 does not seems to be related to Spatial Navigation. ChangeLog has some minor typos, spacing issues, please fix it when landing. This patch looks ok to me, but I'd like to give some time to Cary to give feedback before giving an r+. I do not think we have enough evidence that this flag is not used by any ports. The fact that this is not listed in Platform.h is not enough evidence. 208878 4 tonikitoo 2010-04-05 20:43:05 -0700 (In reply to comment #3) > Bug 18862 does not seems to be related to Spatial Navigation. err, it is bug 18662. Sorry. > ChangeLog has some minor typos, spacing issues, please fix it when landing. sure, thank you. > This patch looks ok to me, but I'd like to give some time to Cary to give > feedback before giving an r+. I do not think we have enough evidence that this > flag is not used by any ports. The fact that this is not listed in Platform.h > is not enough evidence. cary, eric, what do you guys think? 209002 5 caryclark 2010-04-06 05:49:24 -0700 The DIRECTIONAL_PAD_NAVIGATION condition is obsolete. D-pad navigation on Android used to change the DOM focus; now, it only changes the mouse position. Since the focus is only set when the page initiates Javascript, or through mouse click events, this scroll in response to changing focus no longer needs to be suppressed. Android separately ignores scroll requests that are not initiated by user actions, so it's fine to remove this. 209003 6 tonikitoo 2010-04-06 05:54:40 -0700 (In reply to comment #5) > The DIRECTIONAL_PAD_NAVIGATION condition is obsolete. D-pad navigation on > Android used to change the DOM focus; now, it only changes the mouse position. > Since the focus is only set when the page initiates Javascript, or through > mouse click events, this scroll in response to changing focus no longer needs > to be suppressed. > > Android separately ignores scroll requests that are not initiated by user > actions, so it's fine to remove this. thank you Cary. Laszlo, what typos you referred to? 209028 7 52601 laszlo.gombos 2010-04-06 06:55:59 -0700 Comment on attachment 52601 (committed in r57148) patch LGTM based on Cary's comments. The typo seems to be only in the bug report and not in ChangeLog. 209042 8 52601 tonikitoo 2010-04-06 07:33:44 -0700 Comment on attachment 52601 (committed in r57148) patch thanks laszlo 52601 2010-04-05 20:23:17 -0700 2010-04-06 07:33:43 -0700 (committed in r57148) patch 0001-Remove-unused-DIRECTIONAL_PAD_NAVIGATION.patch text/plain 2633 tonikitoo RnJvbSA2MWYyMDZiOGQ0ZTk2NDQ0NWM5ZmZlYjNiZWJjN2Y4NmM4OTM0ZTQzIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBBbnRvbmlvIEdvbWVzIDx0b25pa2l0b29Ad2Via2l0Lm9yZz4K RGF0ZTogTW9uLCA1IEFwciAyMDEwIDIzOjIyOjA3IC0wNDAwClN1YmplY3Q6IFtQQVRDSF0gUmVt b3ZlIHVudXNlZCBESVJFQ1RJT05BTF9QQURfTkFWSUdBVElPTgogaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTM3MTM0CgpSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K UGF0Y2ggYnkgQW50b25pbyBHb21lcyA8dG9uaWtpdG9vQHdlYmtpdC5vcmc+CgpBbmRyb2lkIGJy b3dzZXIgd2FzIHBsYW5uaW5nIHRvIGhhdmUgYSAiZGlyZWN0aW9uYWwgcGFkIG5hdmlnYXRpb24i IGF2YWlsYWJsZSBvbgp0cnVuaywgdW5kZXIgYSBidWlsZCBndWFyZCBjYWxsZWQgRElSRUNUSU9O QUxfUEFEX05BVklHQVRJT04sIGJ1dCBhY2NvcmRpbmcgdG8KaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTIzMTQ1I2MzICwgdGhlIHBsYW4gd2FzIGRyb3BwZWQuCgpIb3dl dmVyIGJ1ZyAyMzE2MyBsYW5kZWQgc29tZSBjb2RlIHJlbHlpbmcgb24gdGhlIGJ1aWxkIGZsYWcg c3VwcG9zZWRseSB0byBiZSBhZGRlZCwKYW5kIGlzIG5vdyBkZWFkIGNvZGUgaW4gdHJ1bmsuCgoq IGRvbS9FbGVtZW50LmNwcDoKKFdlYkNvcmU6OkVsZW1lbnQ6OnVwZGF0ZUZvY3VzQXBwZWFyYW5j ZSk6Ci0tLQogV2ViQ29yZS9DaGFuZ2VMb2cgICAgICAgfCAgIDE3ICsrKysrKysrKysrKysrKysr CiBXZWJDb3JlL2RvbS9FbGVtZW50LmNwcCB8ICAgIDcgKy0tLS0tLQogMiBmaWxlcyBjaGFuZ2Vk LCAxOCBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL1dlYkNvcmUv Q2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOWE0ZGIyNi4uOTA0NjZlNyAxMDA2 NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs NSArMSwyMiBAQAogMjAxMC0wNC0wNSAgQW50b25pbyBHb21lcyAgPHRvbmlraXRvb0B3ZWJraXQu b3JnPgogCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJl bW92ZSB1bnVzZWQgRElSRUNUSU9OQUxfUEFEX05BVklHQVRJT04KKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM3MTM0CisKKyAgICAgICAgQW5kcm9pZCBi cm93c2VyIHdhcyBwbGFubmluZyB0byBoYXZlIGEgImRpcmVjdGlvbmFsIHBhZCBuYXZpZ2F0aW9u IiBhdmFpbGFibGUgb24KKyAgICAgICAgdHJ1bmssIHVuZGVyIGEgYnVpbGQgZ3VhcmQgY2FsbGVk IERJUkVDVElPTkFMX1BBRF9OQVZJR0FUSU9OLCBidXQgYWNjb3JkaW5nIHRvCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMzE0NSNjMyAsIHRoZSBwbGFu IHdhcyBkcm9wcGVkLgorCisgICAgICAgIEhvd2V2ZXIgYnVnIDIzMTYzIGxhbmRlZCBzb21lIGNv ZGUgcmVseWluZyBvbiB0aGUgYnVpbGQgZmxhZyBzdXBwb3NlZGx5IHRvIGJlIGFkZGVkLAorICAg ICAgICBhbmQgaXMgbm93IGRlYWQgY29kZSBpbiB0cnVuay4KKworICAgICAgICAqIGRvbS9FbGVt ZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkVsZW1lbnQ6OnVwZGF0ZUZvY3VzQXBwZWFyYW5j ZSk6CisKKzIwMTAtMDQtMDUgIEFudG9uaW8gR29tZXMgIDx0b25pa2l0b29Ad2Via2l0Lm9yZz4K KwogICAgICAgICBSZXZpZXdlZCBieSBMYXN6bG8gR29tYm9zLgogCiAgICAgICAgIENsZWFuZWQg dXAgc3BhdGlhbC1uYXZpZ2F0aW9uLXRlc3QtY2FzZXMuaHRtbCBieSByZW1vdmluZyB0aGUgd3Jv bmdseQpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9kb20vRWxlbWVudC5jcHAgYi9XZWJDb3JlL2RvbS9F bGVtZW50LmNwcAppbmRleCAxYjBjMDEyLi4yMTBlNGRjIDEwMDY0NAotLS0gYS9XZWJDb3JlL2Rv bS9FbGVtZW50LmNwcAorKysgYi9XZWJDb3JlL2RvbS9FbGVtZW50LmNwcApAQCAtMTMzNywxMyAr MTMzNyw4IEBAIHZvaWQgRWxlbWVudDo6dXBkYXRlRm9jdXNBcHBlYXJhbmNlKGJvb2wgLypyZXN0 b3JlUHJldmlvdXNTZWxlY3Rpb24qLykKICAgICAgICAgICAgIGZyYW1lLT5zZWxlY3Rpb24oKS0+ c2V0U2VsZWN0aW9uKG5ld1NlbGVjdGlvbik7CiAgICAgICAgICAgICBmcmFtZS0+cmV2ZWFsU2Vs ZWN0aW9uKCk7CiAgICAgICAgIH0KLSAgICB9Ci0gICAgLy8gRklYTUU6IEknbSBub3Qgc3VyZSBh bGwgZGV2aWNlcyB3aWxsIHdhbnQgdGhpcyBvZmYsIGJ1dCB0aGlzIGlzCi0gICAgLy8gY3VycmVu dGx5IHR1cm5lZCBvZmYgZm9yIEFuZHJvaWQuCi0jaWYgIUVOQUJMRShESVJFQ1RJT05BTF9QQURf TkFWSUdBVElPTikKLSAgICBlbHNlIGlmIChyZW5kZXJlcigpICYmICFyZW5kZXJlcigpLT5pc1dp ZGdldCgpKQorICAgIH0gZWxzZSBpZiAocmVuZGVyZXIoKSAmJiAhcmVuZGVyZXIoKS0+aXNXaWRn ZXQoKSkKICAgICAgICAgcmVuZGVyZXIoKS0+ZW5jbG9zaW5nTGF5ZXIoKS0+c2Nyb2xsUmVjdFRv VmlzaWJsZShnZXRSZWN0KCkpOwotI2VuZGlmCiB9CiAKIHZvaWQgRWxlbWVudDo6Ymx1cigpCi0t IAoxLjYuMC40Cgo=