Closed Bug 1337025 Opened 8 years ago Closed 7 years ago

Clicking the "x" does not repopulate the list

Categories

(Core :: Layout: Form Controls, defect)

53 Branch
All
Unspecified
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: jwilliams, Assigned: saghan99, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 file, 1 obsolete file)

STR:
1. Open Search Form (Select Test).html
2. Select "Very Long Search Over" 1000 Items
3. Click in the search option
4. type "0"
5. Click the "x"

Expected Result:
The list should repopulate

Actual Result:
The list stays the same with all items containing a "0"

I can reproduce this on Win 10 machine & Touchscreen Laptop, Ubuntu 16.04, & Mac 10.12
Version: 54 Branch → 53 Branch
Blocks: 1332301
This should be a simple bug to fix, probably a good first bug for someone who is tackling getting bug 1332301 fixed.

Note, you'll need to enable this feature to work on this bug. It is enabled by visiting about:config and setting dom.forms.selectSearch to true.

You can use any of the dropdowns on http://webdev.cse.msu.edu/~beachjar/capstone/test.html that have over 40 items.
Mentor: jaws
Whiteboard: [good first bug][lang=js]
Looking at the _clearSearch function in textbox.xml, http://searchfox.org/mozilla-central/rev/7cb75d87753de9103253e34bc85592e26378f506/toolkit/content/widgets/textbox.xml#406, when the search is cleared the 'command' event is fired. (We should also fire the 'input' event but we're not right now). The simplest solution here would be to add another event listener for "command" and use onSearchInput with that event too.

We can add the line below http://searchfox.org/mozilla-central/rev/7cb75d87753de9103253e34bc85592e26378f506/toolkit/modules/SelectParentHelper.jsm#328 to add an event listener for "command".
Hi Jared,

I am a first time contributor to Mozilla and Id like to work on this ticket as its marked for "Good First Bug".

Please let me know if I can get started on this. And would you mind giving me the instructions on how to get set up with the repo/workflow?

Regards,
Ganesh
Yes, you can get started on this. I will wait until a patch is uploaded before marking this as assigned. You can follow the steps at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build to learn how to set up a Firefox build environment. The steps outlined in comment #2 of this bug will help you to fix the bug.
Sure,Ill start with setting up the build environment. Will get back to you if I face any issues.
Hey Jared,

I was getting the below error when I was trying to setup by running start-shell-msvc2015.bat:

C:\mozilla-build>start-shell-msvc2015.bat
ERROR: Cannot determine the location of the VS Common Tools folder.
ERROR: Cannot determine the location of the VS Common Tools folder.
MozillaBuild Install Directory: C:\mozilla-build\
Visual C++ 2015 Directory: C:\Program Files (x86)\Microsoft Visual Studio\Shared
\14.0\VC\
Windows SDK Directory: C:\Program Files (x86)\Windows Kits\8.1\
Trying to use the MSVC 2015 32-bit toolchain.

Unable to call a suitable vcvars script. Exiting.

Press any key to continue . . .

I'm using a windows 8.1 machine and I have installed VC++ 2015.3 v 140 toolset from the Visual Studio Community 2017. Can you please help me in figuring out what went wrong ?
Can you join the #introduction channel on the Mozilla IRC server? People there will be able to help you get your build set up faster than going through Bugzilla. Here is a link to the channel using Mibbit, http://chat.mibbit.com/?channel=%23introduction&server=irc.mozilla.org

You should look at using an Artifact build, as that has less dependencies and will build *much* faster, https://developer.mozilla.org/en-US/docs/Artifact_builds
Hi Jared,
 I would like to work on this bug. I am new to contributing to firefox. I have a few questions:
1. Is the name of the event that gets fired when user clicks 'x' in search called 'command'?
2. The "_clearSearch" method defined in textbox.xml should call onSearchInput() from SelectParentHelper.jsm after the line this.value = ""; is executed which clears the data in search box. How do I call method from js file from method in xml file?   
According to your suggestion, a way to fix this is by adding a statement in SelectParentHelper.jsm :
    searchbox.addEventListener("command", newFunction);
The newFunction would clear the text in searchbox and call onSearchInput(). Is that right?
Also, a weird thing happens that if I create console.log('test') in 1st line of onSearchInput() nothing gets printed on console and the search functionality does not work.
-Thanks for your help
Flags: needinfo?(jaws)
Attached patch patch to fix (deleted) — Splinter Review
I fixed the bug. Calling the onSearch() function again after "command" is fired repopulates the list.
Attachment #8848362 - Flags: review?(jaws)
Attached patch Patch with the fix to the bug. (obsolete) (deleted) — Splinter Review
Hi Jared,

I was able to build using the artifacts. That page was really helpful. I was able to fix the bug of course its similar to one in comment 9. Attaching the fix, please review it and once the bug is assigned to me and review is done Ill push the changes.
Attachment #8848898 - Flags: review?(jaws)
Comment on attachment 8848362 [details] [diff] [review]
patch to fix

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

Thanks, this patch will work.
Attachment #8848362 - Flags: review?(jaws) → review+
Assignee: nobody → saghan99
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Comment on attachment 8848898 [details] [diff] [review]
Patch with the fix to the bug.

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

Sorry, Saghan sent in a patch first so I decided to just go with what was submitted first. This patch is close, but there are some issues with indentation.
Attachment #8848898 - Flags: review?(jaws)
Attachment #8848898 - Attachment is obsolete: true
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab1bd5290976
Add event listener to command event which will be triggered when x button is clicked and repopulate the list. r=jaws
Keywords: checkin-needed
Hi Jared,
 I see pulsebot has pushed the patch for integration. Is there anything more to do about this bug from my end?
-Warm Regards
Flags: needinfo?(jaws)
(In reply to Saghan from comment #14)
> Hi Jared,
>  I see pulsebot has pushed the patch for integration. Is there anything more
> to do about this bug from my end?
> -Warm Regards

Hi Saghan, now that the patch has been pushed to autoland all work is done here. The patch should get merged to mozilla-central within a day or two and then it will appear in Firefox Nightly builds the day after.
Flags: needinfo?(jaws)
https://hg.mozilla.org/mozilla-central/rev/ab1bd5290976
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
I can not verify this fix as it depends on bug 1343569.
Depends on: 1343569
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: