464598 - SNAV: snav can move focus out from single-line input fields only if cursor is at one of the widget edges

Status: RESOLVED INCOMPLETE

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Antonio Gomes (tonikitoo)]

Attachment: simple testcase

STEPS:
1) Open the attached test-case.
2) put focus in the single-line form input.
3) type any text (more than 3 characters for easier reproducing).
4) w/out moving focus out from the form, put cursor in the middle of the typed word (it can be any position different from the first and last positions).
5) press keyDown to move focus down through snav.

CURRENT OUTCOME:
it does not move focus down.

EXPECTED OUTCOME:
focus moved away from single-line input and put in multi-line input

Comment 1

[Antonio Gomes (tonikitoo)]

Attachment: handle singleline and multiline input separately.

we got better behaviour if we handle single and multiline inputs on their own.

Comment 2

[Antonio Gomes (tonikitoo)]

Attachment: handle singleline and multiline inputs separately (v2)

fixed typos

Comment 3

[Doug Turner (:dougt)]

lets get some tests case for this.

Comment 4

[Antonio Gomes (tonikitoo)]

ok , I will make 

chrome://mochikit/content/chrome/toolkit/spatial-navigation/tests/test_snav_textFields.xul 

to run w/out failures according to the patch.

Comment 5

[Antonio Gomes (tonikitoo)]

Attachment: handle singleline and multiline inputs separately (v3)

added changes in toolkit/spatial-navigation/tests/chrome/test_snav_textFields.xul

accordingly to the changes in patch #2.

tests all passed : 59/59

Comment 6

[Antonio Gomes (tonikitoo)]

changes applyable after changes in bug 463514.

mark, doug asked for someone else to check that. I would be glad if you could...

Comment 7

[Antonio Gomes (tonikitoo)]

re-ping review ...

Comment 8

[Antonio Gomes (tonikitoo)]

Attachment: handle singleline and multiline inputs separately (v4)

Comment 9

[Doug Turner (:dougt)]

Comment on attachment 357970 [details] [diff] [review]
handle singleline and multiline inputs separately (v4)

+        // we are at the start of the text, okay to move
+          if (target.selectionStart != 0)


change this comment to read the:

if there is text, there it isn't okay to move

same here:

+        // we are at the start of the text, okay to move
+          if (target.selectionStart != 0)


If we are not at the start of the text, it isn't okay to move.


-<window title="Mozilla Bug 288254"
+<window title="Mozilla Bug 436084"

why this change.

With those fixes, and a reasonable explanation for the window title change, r=.

Comment 10

[Antonio Gomes (tonikitoo)]

Attachment: handle singleline and multiline inputs separately (v5)

(In reply to comment #9)
> (From update of attachment 357970 [details] [diff] [review])
> +        // we are at the start of the text, okay to move
> +          if (target.selectionStart != 0)
>
>
> change this comment to read the:
> if there is text, there it isn't okay to move

ok

> same here:
> +        // we are at the start of the text, okay to move
> +          if (target.selectionStart != 0)
> If we are not at the start of the text, it isn't okay to move.

ok
 
> -<window title="Mozilla Bug 288254"
> +<window title="Mozilla Bug 436084"

it is probably a c&p typo. look:

"Bug 288254 -  (xblfindbar) Findbar XBL Widget " has nothing to do with our current bug.

Comment 11

[Antonio Gomes (tonikitoo)]

Comment on attachment 358046 [details] [diff] [review]
handle singleline and multiline inputs separately (v5)

carry dougt's r+

Comment 12

[Antonio Gomes (tonikitoo)]

Attachment: handle singleline and multiline inputs separately (v6)

v6.

the v5 patch mochitest part failed on MAC because it made use of HOME/END keys, which are not available in this platform.

patch is the same as previous but fixed this issue.

Comment 13

[Antonio Gomes (tonikitoo)]

fixed

85b9917cf2e3

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Backed out because the new test was causing a lot of failures on Mac OS X:
http://hg.mozilla.org/mozilla-central/rev/766303321a07
http://hg.mozilla.org/mozilla-central/rev/75de262e2a2e

Comment 15

[Antonio Gomes (tonikitoo)]

doug, test is failing on MAC (and has been reopened twice) but I do not have a MAC box handy to make it work on that platform.

Could you help me testing it on that platform please ?

Comment 16

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This bug's parent component is going away. Should this bug be moved to an equivalent component in Android or iOS?

Comment 17

[Kevin Brosnan [Ex-Mozilla]]

This is a Gecko bug thus irrelevant for Firefox for iOS. I don't know of an Android device with a directional pad that we support (Android 4.0+).