305342 - regression: tabbing from links doens't work in FAYT mode

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect, P1)

Description

[Asaf Romano (gone)]

Tabbing to links doens't work anymore in FAYT mode (instead, it tabs inside the
findbar or something).

It looks related to the focus changes which were done in order to support IME,
but I think it regressed a bit later.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Do you think that if found link has pseudo focus, we should be able to change
the focus in links by tab?

Comment 2

[Asaf Romano (gone)]

That's the way FAYT used to work, yes.

(Also, I think you want to go back to "real" focus once you press the TAB key, I
hope it doesn't break IME).

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Should I stop FAYT mode if Tab key is pressed?
Or should keep FAYT mode?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Umm.. Maybe, I cannot keep FAYT mode after Tab. Because if we keep it, we need
to emulate the focus changing mechanism. But it is very difficult. The focusable
element is not links only. e.g., input element, select element...

Most easy approach is:

1. Stop FAYT mode.
2. Set focus to found link.
3. Fire tab key event on browser.(I don't know it is possible or not.)

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I will work on this tomorrow.(I will sleep, sorry.)

Comment 6

[Asaf Romano (gone)]

let's stop FAYT at that poiunt.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

This patch finish the FAYT mode when user type Tab or Shift+Tab and a link is
found.

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 193367 [details] [diff] [review]
Patch rv1.0

>+function finishFAYT(keypressEvent)

please use aKeypressEvent here

>+  else if (evt.keyCode == KeyEvent.DOM_VK_TAB) {
>+    var shouldHandle = !evt.altKey && !evt.ctrlKey && !evt.metaKey;
>+    if (shouldHandle && gFindMode != FIND_NORMAL && gFoundLink) {
>+      if (finishFAYT(evt)) {

you can save a level of nesting by adding the finishFAYT() call into the parent
if statement.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch for check-in

The risk is low. This is regression bug.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in to branch too.

Comment 12

[Asaf Romano (gone)]

verified.

Many thanks, Masayuki!