1032724 - Improve spatial navigation

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Comment 1

[Kershaw Chang [:kershaw]]

I've observed some strange behaviors of the current spatial navigation by the test page http://jsfiddle.net/LECVP/8/show/.

Take a look at the following two diagrams. A is the current focused element. 
When pressing down key,
expect: B should be focused.
actual: C is focused.

1. 
+---+      
| A | <-- the current focused element
+---+      
+---+ +---+
| B | | C |
+---+ +---+
2.
+-----+                                     
|  A  | <-- the current focused element                                  
+-----+                                                                           
+------------------------------------------+
|                                          |
|                    B                     |
|                                          |
+-----+------------------------------------+
+-----+                                     
|  C  |                                     
+-----+

Comment 2

[Kershaw Chang [:kershaw]]

Add the third case.
3. B and C are all below A. When pressing down key, B should be focused, not C.
+---+                              
| A | <-- the current focused element                              
+---+                              
                              +---+
                              | B |
                              +---+            
+---+                              
| C |                              
+---+

Comment 3

[Kershaw Chang [:kershaw]]

Hi Brad,

First, I am not sure if you are the one to talk about spatial navigation. If not, would you please help me to get the correct one involved?
I would like to make sure if this is really a bug. Have you seen the same cases I mentioned above?

Thanks.
Kershaw

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Kershaw Chang [:kershaw] from comment #3)
> Hi Brad,
> 
> First, I am not sure if you are the one to talk about spatial navigation. If
> not, would you please help me to get the correct one involved?
> I would like to make sure if this is really a bug. Have you seen the same
> cases I mentioned above?
> 
> Thanks.
> Kershaw

I have not tested spatial navigation in almost a year. What you describe sounds like a bug. Can you attach a test case?

Comment 5

[Kershaw Chang [:kershaw]]

Attachment: test_spatial_navigation_kershaw.html

Upload a test case which shows the problem I mentioned in case1.

Comment 6

[Kershaw Chang [:kershaw]]

Attachment: Proposed patch for improving spatial navigation

Hi Brad,

I've uploaded my proposed patch. Would you help to review it?

Thanks.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

Hi Kershaw, I've twice started to reivew this patch but have been pulled off for other things. Unfortunitely it takes a little extra time since this isn't code that I've been looking at a lot lately. I should have time to get this review done tomorrow though.

Comment 8

[Kershaw Chang [:kershaw]]

Hi Brad,
No problem! Take your time.
Really thanks for your help.

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8452192 [details] [diff] [review]
Proposed patch for improving spatial navigation

Review of attachment 8452192 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/modules/tests/mochitest/test_spatial_navigation.html
@@ +43,5 @@
>         "Spatial navigation up key is not handled correctly.");
>  
>      center.focus();
>      synthesizeKey("VK_DOWN", { });
> +    is(document.activeElement.id, bottom.id, 

nit, trailing white space (here and futher on in the patch)

Comment 10

[Kershaw Chang [:kershaw]]

Attachment: [Final]Improve spatial navigation

Update the final patch including the following changes:
1. Remove trailing spaces
2. Carry reviewer's name

Comment 11

[Kershaw Chang [:kershaw]]

https://tbpl.mozilla.org/?tree=Try&rev=af7d8b904678

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/b6ca88f40b79

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b6ca88f40b79