Tapping under the last option in a combo box will select the last option and deselect all the others

RESOLVED FIXED in Firefox 42

Status

()

Firefox for Android
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Flaviu Cos, Assigned: capella, Mentored)

Tracking

Trunk
Firefox 42
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox35 affected, firefox36 affected, firefox37 affected, firefox38 affected, firefox39 affected, firefox40 affected, firefox41 affected, firefox42 fixed, fennec+)

Details

(Whiteboard: [lang=java][lang=js])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Environment: 
Device: Samsung Galaxy S4 (Android 4.4.2);
Build: Nightly 38.0a1 (2015-01-14);

Steps to reproduce:
1. Go to bugzilla.mozilla.org;
2. Tap on Search button;
3. Tap on Advanced Search;
4. Tap on the first combo box;
5. Select some of the available options from the combo box;
6. Tap on Done button;
7. Tap again on the combo box but this time on the empty space under the last option.

Expected result:
The combo box is opened and the options selected at step 5 are selected.

Actual result:
The combo box is opened and only the last option is selected. 

Notes:
Please check the video: http://youtu.be/ViCQ8ZP7YC0
It would be good to test this on combo boxes that are on other sites. Maybe it requires a scrollable list?
tracking-fennec: --- → ?
Flags: needinfo?(flaviu.cos)
(Reporter)

Comment 2

3 years ago
(In reply to Kevin Brosnan [:kbrosnan] from comment #1)
> It would be good to test this on combo boxes that are on other sites. Maybe
> it requires a scrollable list?

The issue is reproducible also on http://www.gpo.gov/fdsys/search/advanced/advsearchpage.action
Flags: needinfo?(flaviu.cos)
Mentor: wjohnston@mozilla.com
tracking-fennec: ? → +
Whiteboard: [lang=java][lang=js]

Comment 3

3 years ago
I can take this up, if it's available. I'm not yet sure how to go about fixing it, but I can start looking at it. If you can provide some pointers, that would probably save me some time.
(Reporter)

Updated

3 years ago
status-firefox39: --- → affected
(Reporter)

Updated

3 years ago
status-firefox40: --- → affected
(Reporter)

Updated

3 years ago
status-firefox41: --- → affected
(Reporter)

Updated

3 years ago
status-firefox42: --- → affected
(Assignee)

Comment 4

3 years ago
This is interesting ...

   Mobile behaves this way, because that's how core Gecko is designed. Using [0] as a testcase on desktop or Mobile, you can see that the code at [1] returns the last item in the list on mouseclick into empty space.

   This seemingly sets |element.selectedIndex|, before firing the element.onClick() event where we notice it via listener on the Mobile side [2].

   Chrome desktop however handles this differently, and selects / returns nothing. Complicating things somewhat, it finesses the situation on Mobile by changing the basic element rendering [3].

   Sooooooooo, not sure if this is evangelism / something we can change in core so mobile "just works", something that is truly a Mobile only fix, or a combination of both sides.

   I'm poking roc for ni? as I see his name as previous reviewer of |nsListControlFrame.cpp| ...

[0] https://www.dropbox.com/s/ukvqqq7mq4mglyw/test_bug1121515.html?dl=0
[1] http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsListControlFrame.cpp?rev=79800010be78&mark=1775-1776,1783-1783#1725
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=057f0bd4075c&mark=5114-5114#5099
[3] https://www.dropbox.com/s/ejbgq6hr8w1dus4/test_bug1121515.mp4?dl=0
Flags: needinfo?(roc)
I don't know the mobile combobox code. I'm surprised that that line of code is hit at all on mobile. Maybe we should remove it on mobile on desktop; I don't think it's actually helpful on desktop.
Flags: needinfo?(roc)
(Assignee)

Comment 7

3 years ago
Created attachment 8632437 [details] [diff] [review]
bug1121515.diff

That works for me!

This provides Chrome desktop compatibility, and solves the issue on the Android side.

If you can signoff on desktop, I'll get wesj to approve Mobile next.
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #8632437 - Flags: review?(roc)
Comment on attachment 8632437 [details] [diff] [review]
bug1121515.diff

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

For consistency we should probably eliminate the "is the event above the first option" code too.
(Assignee)

Comment 9

3 years ago
Created attachment 8632438 [details] [diff] [review]
bug1121515.diff

(wondered)

Ok, re-tested both builds locally. I'll get an insurance TRY push after all review?'s.
Attachment #8632437 - Attachment is obsolete: true
Attachment #8632437 - Flags: review?(roc)
Attachment #8632438 - Flags: review?(roc)
(Assignee)

Comment 10

3 years ago
Created attachment 8632513 [details] [diff] [review]
bug1121515_testUpdate.diff

Update existing test to correct expectations, push all to TRY.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aca0f3886e57
Attachment #8632513 - Flags: review?(roc)
(Assignee)

Updated

3 years ago
Attachment #8632438 - Flags: review?(wjohnston)
Attachment #8632438 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/d0590592a75b
https://hg.mozilla.org/mozilla-central/rev/51c6347fe448
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.