1337025 - Clicking the "x" does not repopulate the list

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Justin [:JW_SoftvisionQA]]

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

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

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".

Comment 3

[Ganesh Chaitanya Kale]

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

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 5

[Ganesh Chaitanya Kale]

Sure,Ill start with setting up the build environment. Will get back to you if I face any issues.

Comment 6

[Ganesh Chaitanya Kale]

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 ?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 8

[Saghan]

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

Comment 9

[Saghan]

Attachment: patch to fix

I fixed the bug. Calling the onSearch() function again after "command" is fired repopulates the list.

Comment 10

[Ganesh Chaitanya Kale]

Attachment: Patch with the fix to the bug.

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.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8848362 [details] [diff] [review]
patch to fix

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

Thanks, this patch will work.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

[Saghan]

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

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

(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.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ab1bd5290976

Comment 17

[Justin [:JW_SoftvisionQA]]

I can not verify this fix as it depends on bug 1343569.