592252 - Restart is required when pressing the down arrow in localization and autodetect is selected already

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Ioana Chiorean]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100829 Firefox/4.0b5pre
Build Identifier: Mozilla/5.0 (Maemo; Linux armv7l; rv:2.0b5pre) Gecko/20100831 Firefox/4.0b5pre Fennec/2.0b1pre

- when pressing the down arrow , a drop down list should appear highlighting the language that is chosen
- for the case of having already autodetect, when pressing the down arrow it also requires a restart
- if you continue and press other language than auto detect, the restart requirement doesn't chance the language, but the selected one is installed after restart

Reproducible: Always

Steps to Reproduce:
1. Instal multilanguage Fennec
2. Webpage to the left to open the controls sidebar
3. Click on options (gears) icon 
3. At language press the down arrow  (by default is chosen auto detect)
Actual Results:  
- the application highlights autodetect and requires in background to restart 



Expected Results:  
- the aplication should not ask directly to restart just after choosing a language

Comment 1

[Ioana Chiorean]

Attachment: screenshots

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

creating a ZIP of screenshots is a bit of a pain to view. try to just add them as attachments rather than an archive.

I assume this behavior is happening because lists are designed for tapping with fingers, and when an item is tapped the onselect or oncommand is firing.

When using an arrow key, the selection is changing for each key press, causing the action to fire too.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

I have fully sure to understand what is the bug here.
Is it that the "Restart" prompt use the first selected language in the list and never change even if you use choose another language? (this is a bug but I want to be sure we agree on the bug)

Comment 4

[Ioana Chiorean]

Yes Vivien, the prompt appear before choosing a language, in English (for having already autodetect) and remains in english even if you choose other language and the expected behavior is first to choose a language and after the prompt to appear translated in the selected language

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #4)
> Yes Vivien, the prompt appear before choosing a language, in English (for
> having already autodetect) and remains in english even if you choose other
> language and the expected behavior is first to choose a language and after the
> prompt to appear translated in the selected language

Hey Ioana,

I see the second part of the bug (the wrong language) but I'm not able to see the first part which is, if I undestand correcly, that the "Restart" prompt pop up before you have done a choice?

/me which have hard times reproducing Ioana's bugs bug know they exists!

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #4)
> Yes Vivien, the prompt appear before choosing a language, in English (for
> having already autodetect) and remains in english even if you choose other
> language and the expected behavior is first to choose a language and after the
> prompt to appear translated in the selected language

Just a clarification: The language will not actually change until we restart. We can't start displaying text in a different language until the application restarts.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Stealing...

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

I found two problems:
1. the currentLocale we compare against was not set to "suto" if we were in auto-detect mode
2. the menulist is split into a dropmark (button) and a list of menuitems. Clicking the dropmarker will fire a "command" event, as will the menuitems.

#2 is the biggest problem since the XUL sets an oncommand listener for the menulist. We do this because onselect will fire even when the list is being populated and initialized. However, pressing the dropmark (arrow) will fire the oncommand handler far too soon and unexpectedly.

The patch tweaks the "command" handler to only pass "command" events from the menuitems through the menulist.

Comment 9

[Matt Brubeck (:mbrubeck)]

Comment on attachment 482774 [details] [diff] [review]
patch

>+++ b/chrome/content/bindings.xml

>+          if (event.target.parentNode.parentNode == this)
>+            this.selectedItem = event.target;
>+          else
>+            event.stopPropagation();

Is |this.selectedItem = event.target;| necessary?  I think that will happen automatically if we just let the event propagate.

r+ but remove the first branch of the "if" unless I'm missing something.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

removed the first branch of the "if" - it wasn't needed

pushed:
http://hg.mozilla.org/mobile-browser/rev/1bede1cdff62

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Verified:

Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b8pre) Gecko/20101014 Firefox/4.0b8pre Fennec/4.0b2pre
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101014 Firefox/4.0b8pre Fennec/4.0b2pre

Comment 12

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=13836
https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=13835