Closed Bug 715990 Opened 12 years ago Closed 3 years ago

Fire click events on option elements

Categories

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

defect

Tracking

(fennec+)

RESOLVED INCOMPLETE
Tracking Status
fennec + ---

People

(Reporter: wesj, Unassigned)

Details

(Whiteboard: [lang=js])

Attachments

(1 file)

bug 715303 set us up to fire focus on selects when they are clicked. We apparently in xul fennec also fire mouse events on option elements. We should try to reproduce the old behavior if possible.
Assignee: nobody → wjohnston
tracking-fennec: --- → 11+
Priority: -- → P3
tracking-fennec: 11+ → +
Assignee: wjohnston → nobody
Whiteboard: [mentor=wesj][lang=js]
Hi. I would like to help closing this bug. Which page can I try and reproduce the bug?
Flags: needinfo?(wjohnston)
No problem. I don't think we have a test page for this though :) It should be simple to write one. Just make a generic html page with a select element in it:

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select?redirectlocale=en-US&redirectslug=HTML%2FElement%2Fselect

add an onclick="alert('foo');" attribute on one (or all) of the option elements and then select it. We should probably fire click's right next to this place where we fire "change" events: http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/SelectHelper.js
Flags: needinfo?(wjohnston)
I'd like to submit a patch for this bug. Apart from the patch to SelectHelper, where in the source tree would a test be added?
Flags: needinfo?(wjohnston)
We don't have good tests for these right now :( I'd be happy to take an initial simple one to get things started.

Most of our tests for this kind of thing would go in mobile/android/base/tests. You'd need to create a java file there that would load an HTML file with select elements in it. This will be a bit tricky because the assertions need to happen in Java. We do tricks in some places like changing the page title and listening for those events in Java, or showing an Alert with some text we can watch for.

Alternatively, you can load a page from the mobile/android/base/roboextender as a chrome page (which can then send messages to java. I've been playing with that approach in bug 980074.
Flags: needinfo?(wjohnston)
Thanks! Have a flight tomorrow so I'll keep myself busy working on this.
Simple patch for the actual dispatching of click events. Now I gotta figure out how to write a useful test...

(just useing https://miketaylr.com/bzla/715990.html for local manual testing)
Comment on attachment 8392002 [details] [diff] [review]
bug-715990-fix.patch

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

Thanks! and Whoops! I apologize for missing this. When you upload a patch, there's a review element in the upload form. Make sure you flip it so that things appear in my review dashboard. I think this looks good with one little change. Can you upload another patch?

::: mobile/android/chrome/content/SelectHelper.js
@@ +57,5 @@
>            }
>            i++;
>          });
>  
> +        this.fireOnClick(aElement.children[aElement.selectedIndex]);

This probably needs to be inside the if (change) line below it. i.e. We don't need to fire these if the item hasn't been flipped.
No need for an apology--it was more of an uploading a WIP thing before I get to testing (hopefully in the next few days). Thanks!
Mentor: wjohnston
Whiteboard: [mentor=wesj][lang=js] → [lang=js]
Is this still relevant, and can I farm it out to a community member if so?
Flags: needinfo?(wjohnston)
Oops, I'm not sure why this disappeared from my radar :/. 

This might be a good first patch if someone wants to tweak the patch I uploaded (last year, wow) and learn the process. Otherwise I can finish this up real quick.
Let me take it from here.
Flags: needinfo?(wjohnston)
Mentor: wjohnston2000
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: P3 → P5
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.