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)
Firefox for Android Graveyard
General
Tracking
(fennec+)
RESOLVED
INCOMPLETE
Tracking | Status | |
---|---|---|
fennec | + | --- |
People
(Reporter: wesj, Unassigned)
Details
(Whiteboard: [lang=js])
Attachments
(1 file)
1.74 KB,
patch
|
Details | Diff | Splinter Review |
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.
Updated•12 years ago
|
Assignee: nobody → wjohnston
tracking-fennec: --- → 11+
Priority: -- → P3
Updated•12 years ago
|
tracking-fennec: 11+ → +
Reporter | ||
Updated•11 years ago
|
Assignee: wjohnston → nobody
Whiteboard: [mentor=wesj][lang=js]
Comment 1•11 years ago
|
||
Hi. I would like to help closing this bug. Which page can I try and reproduce the bug?
Flags: needinfo?(wjohnston)
Reporter | ||
Comment 2•11 years ago
|
||
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)
Comment 3•10 years ago
|
||
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?
Updated•10 years ago
|
Flags: needinfo?(wjohnston)
Reporter | ||
Comment 4•10 years ago
|
||
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)
Comment 5•10 years ago
|
||
Thanks! Have a flight tomorrow so I'll keep myself busy working on this.
Comment 6•10 years ago
|
||
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)
Reporter | ||
Comment 7•10 years ago
|
||
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.
Comment 8•10 years ago
|
||
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!
Assignee | ||
Updated•10 years ago
|
Mentor: wjohnston
Whiteboard: [mentor=wesj][lang=js] → [lang=js]
Comment 9•9 years ago
|
||
Is this still relevant, and can I farm it out to a community member if so?
Flags: needinfo?(wjohnston)
Comment 10•9 years ago
|
||
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.
Comment 12•9 years ago
|
||
Sgtm.
Updated•7 years ago
|
Mentor: wjohnston2000
Comment 13•6 years ago
|
||
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
Comment 14•3 years ago
|
||
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
Assignee | ||
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•