57509 2011-03-30 15:45:34 -0700 Regression: VO cursor doesn't follow KB focus back into HTML view 2011-03-30 17:45:18 -0700 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 cfleizach cfleizach oldest_to_newest 376745 0 cfleizach 2011-03-30 15:45:34 -0700 1. Load attached test case with VO with WK2. 2. Tab into the text field. VO cursor follows. 3. Shift+Tab back to the "new tab" button. VO cursor follows. 4. Tab into the text field. * RESULTS VO cursor stays on the "new tab" button. 376798 1 87641 cfleizach 2011-03-30 17:23:29 -0700 Created attachment 87641 patch 376812 2 87641 darin 2011-03-30 17:37:48 -0700 Comment on attachment 87641 patch View in context: https://bugs.webkit.org/attachment.cgi?id=87641&action=review > Source/WebCore/page/FocusController.cpp:173 > + bool advance = advanceFocus(direction, event, true); I would name the local variable “didAdvanceFocus”. > Source/WebCore/page/FocusController.cpp:179 > + // If focus is being set initially, accessibility needs to be informed that system focus has moved > + // into the web area again, even if focus did not change within WebCore. PostNotification is called instead > + // of handleFocusedUIElementChanged, because this will send the notification even if the element is the same. > + if (AXObjectCache::accessibilityEnabled()) > + focusedOrMainFrame()->document()->axObjectCache()->postNotification(focusedOrMainFrame()->document()->renderer(), AXObjectCache::AXFocusedUIElementChanged, true); Is it correct to do this even if advanceFocus returned false? 376815 3 cfleizach 2011-03-30 17:40:42 -0700 > > Source/WebCore/page/FocusController.cpp:179 > > + // If focus is being set initially, accessibility needs to be informed that system focus has moved > > + // into the web area again, even if focus did not change within WebCore. PostNotification is called instead > > + // of handleFocusedUIElementChanged, because this will send the notification even if the element is the same. > > + if (AXObjectCache::accessibilityEnabled()) > > + focusedOrMainFrame()->document()->axObjectCache()->postNotification(focusedOrMainFrame()->document()->renderer(), AXObjectCache::AXFocusedUIElementChanged, true); > > Is it correct to do this even if advanceFocus returned false? I believe so, because we want to inform that focus is now in the web area, even if it did not move. The consequence of this not being correct is minimal since the focused UI element will be the one that is already focused and the user won't see any difference. 376821 4 cfleizach 2011-03-30 17:45:18 -0700 http://trac.webkit.org/changeset/82525 87641 2011-03-30 17:23:29 -0700 2011-03-30 17:37:48 -0700 patch patch.txt text/plain 2146 cfleizach SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDgyNTI0KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTEtMDMtMzAgIENocmlzIEZs ZWl6YWNoICA8Y2ZsZWl6YWNoQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBSZWdyZXNzaW9uOiBWTyBjdXJzb3IgZG9lc24ndCBmb2xs b3cgS0IgZm9jdXMgYmFjayBpbnRvIEhUTUwgdmlldworICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTc1MDkKKworICAgICAgICBJbiBXSzIsIGJlY2F1c2Ug dGhlIHdlYiBhcmVhIG5ldmVyIGJlbGlldmVzIGZvY3VzIGxlYXZlIHRoZSBhcmVhLCBzZW5kaW5n IHRoZSBpbml0aWFsCisgICAgICAgIGZvY3VzIGNoYW5nZSB3aGVuIG1vdmluZyBiYWNrIGludG8g dGhlIHdlYiBhcmVhLCBkb2VzIG5vdCB0cmlnZ2VyIGEgbm90aWZpY2F0aW9uLgorCisgICAgICAg IFRoYXQgYmVoYXZpb3IgbmVlZHMgdG8gYmUgb3ZlcnJpZGRlbiBieSBleHBsaWNpdGx5IHBvc3Rp bmcgYSBmb2N1c2VkIHVpIGVsZW1lbnQgY2hhbmdlCisgICAgICAgIGF0IHRoZSBhcHByb3ByaWF0 ZSB0aW1lLiBUaGlzIGlzIG9ubHkgdXNlZCBpbiBXSzIsIGhlbmNlIHRoZSBhYnNlbmNlIG9mIGEg bGF5b3V0IHRlc3QgZm9yIG5vdy4KKworICAgICAgICAqIHBhZ2UvRm9jdXNDb250cm9sbGVyLmNw cDoKKyAgICAgICAgKFdlYkNvcmU6OkZvY3VzQ29udHJvbGxlcjo6c2V0SW5pdGlhbEZvY3VzKToK KwogMjAxMS0wMy0zMCAgTU9SSVRBIEhhamltZSAgPG1vcnJpdGFAZ29vZ2xlLmNvbT4KIAogICAg ICAgICBSZXZpZXdlZCBieSBTaW1vbiBGcmFzZXIuCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wYWdl L0ZvY3VzQ29udHJvbGxlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGFnZS9G b2N1c0NvbnRyb2xsZXIuY3BwCShyZXZpc2lvbiA4MjM2OSkKKysrIFNvdXJjZS9XZWJDb3JlL3Bh Z2UvRm9jdXNDb250cm9sbGVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTcwLDcgKzE3MCwxNSBA QAogCiBib29sIEZvY3VzQ29udHJvbGxlcjo6c2V0SW5pdGlhbEZvY3VzKEZvY3VzRGlyZWN0aW9u IGRpcmVjdGlvbiwgS2V5Ym9hcmRFdmVudCogZXZlbnQpCiB7Ci0gICAgcmV0dXJuIGFkdmFuY2VG b2N1cyhkaXJlY3Rpb24sIGV2ZW50LCB0cnVlKTsKKyAgICBib29sIGFkdmFuY2UgPSBhZHZhbmNl Rm9jdXMoZGlyZWN0aW9uLCBldmVudCwgdHJ1ZSk7CisgICAgCisgICAgLy8gSWYgZm9jdXMgaXMg YmVpbmcgc2V0IGluaXRpYWxseSwgYWNjZXNzaWJpbGl0eSBuZWVkcyB0byBiZSBpbmZvcm1lZCB0 aGF0IHN5c3RlbSBmb2N1cyBoYXMgbW92ZWQgCisgICAgLy8gaW50byB0aGUgd2ViIGFyZWEgYWdh aW4sIGV2ZW4gaWYgZm9jdXMgZGlkIG5vdCBjaGFuZ2Ugd2l0aGluIFdlYkNvcmUuIFBvc3ROb3Rp ZmljYXRpb24gaXMgY2FsbGVkIGluc3RlYWQKKyAgICAvLyBvZiBoYW5kbGVGb2N1c2VkVUlFbGVt ZW50Q2hhbmdlZCwgYmVjYXVzZSB0aGlzIHdpbGwgc2VuZCB0aGUgbm90aWZpY2F0aW9uIGV2ZW4g aWYgdGhlIGVsZW1lbnQgaXMgdGhlIHNhbWUuCisgICAgaWYgKEFYT2JqZWN0Q2FjaGU6OmFjY2Vz c2liaWxpdHlFbmFibGVkKCkpCisgICAgICAgIGZvY3VzZWRPck1haW5GcmFtZSgpLT5kb2N1bWVu dCgpLT5heE9iamVjdENhY2hlKCktPnBvc3ROb3RpZmljYXRpb24oZm9jdXNlZE9yTWFpbkZyYW1l KCktPmRvY3VtZW50KCktPnJlbmRlcmVyKCksIEFYT2JqZWN0Q2FjaGU6OkFYRm9jdXNlZFVJRWxl bWVudENoYW5nZWQsIHRydWUpOworCisgICAgcmV0dXJuIGFkdmFuY2U7CiB9CiAKIGJvb2wgRm9j dXNDb250cm9sbGVyOjphZHZhbmNlRm9jdXMoRm9jdXNEaXJlY3Rpb24gZGlyZWN0aW9uLCBLZXli b2FyZEV2ZW50KiBldmVudCwgYm9vbCBpbml0aWFsRm9jdXMpCg==