Closed Bug 1135210 Opened 10 years ago Closed 8 years ago

An empty search on desktop should not say '"" returned X results'

Categories

(Marketplace Graveyard :: Consumer Pages, defect, P2)

Product:
Marketplace Graveyard
Component:
Consumer Pages
Version:
Avenir
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: cvan, Assigned: waseem.tabraze, Mentored)

References

(
URL
)

Details

(Keywords: regression, Whiteboard: [good first bug])

Priority: -- → P3
See Also: → 1129959
Severity: normal → minor
Priority: P3 → P2
Blocks: 1135257
No longer blocks: 1135257
I want to fix this bug.Need Details :)
Flags: needinfo?(ddurst)
(In reply to Atique Ahmed Ziad from comment #1)
> I want to fix this bug.Need Details :)

Great! You'll notice that there are two scenarios here.

In the search box:
1) a search string of 0 length is accepted
2) a search string that contains multiple consecutive spaces (like "   " or "   es") reduces the consecutive spaces to one when the search is executed (so, "   " becomes " ", and "   es" becomes " es"), even though it remains unchanged in the search box

So, a straightforward solution might be to check the string when a user submits it in the search box -- and update it accordingly. This would mean that:
1) a search string of 0 length would simply not execute a search
2) a search string that contains multiple consecutive spaces would reduce the consecutive spaces to one (just as it does now), but also in the search box itself
Flags: needinfo?(ddurst)
Whiteboard: [ktlo]
The previous PR (submitted by Bharath) is closed, this is good to be picked by anyone.
Mentor: vaishnav.rd
Whiteboard: [ktlo] → [ktlo] [good first bug]
Hi Kevin,

Could you please put here some more information about your suggestions given at https://github.com/mozilla/fireplace/pull/1230. The suggestions given on PR may not be sufficient for a newbie here (as this is a good-first-bug).

Thanks,
Ram
Flags: needinfo?(kngo)
I'd just change the displayed search query to "Everything" if there was no input.
Flags: needinfo?(kngo)
I want to work on this.!
Sure Dipankar. Assigning you to this bug.

For any help you can refer to the documentation at http://marketplace.readthedocs.org/, or ask in #marketplace irc channel.

Thanks!
Assignee: nobody → dipankar.sonwane
Status: NEW → ASSIGNED
FTR Dipankar submitted this PR - https://github.com/mozilla/fireplace/pull/1401.

Dipankar, going forward please make sure to put (comment) the PR link on the bug as well.
Dipankar, thank you for the PR. Looks like the patch could be optimized based on the comments. Are you still working on this?
Flags: needinfo?(dipankar.sonwane)
clearing assignee as from the github pull request comments Dipankar isn't working on it anymore.
Assignee: dipankar.sonwane → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(dipankar.sonwane)
I would like to work on this bug . This would be my second good-first-bug.
Flags: needinfo?(vaishnav.rd)
Assigning this bug to Vidhan. Thanks :)
Assignee: nobody → vidhan1995.jain
Status: NEW → ASSIGNED
Flags: needinfo?(vaishnav.rd)
I have generated the pull request.
Link :https://github.com/mozilla/fireplace/pull/1460
Flags: needinfo?(vaishnav.rd)
Thanks Vidhan. NI'ing Kevin for review.
Flags: needinfo?(vaishnav.rd) → needinfo?(kngo)
Flags: needinfo?(kngo)
Hi Vidhan, did you get chance to work on review comments you received on your PR?
Pull request link generated.
https://github.com/mozilla/fireplace/pull/1488
Flags: needinfo?(vaishnav.rd)
Flags: needinfo?(vaishnav.rd) → needinfo?(kngo)
Any progress on this? If this is still unchanged in two weeks, we'll unassign it. (It looks like you got some feedback from cvan, so you can update that PR accordingly.)
Flags: needinfo?(kngo) → needinfo?(vidhan1995.jain)
New PR link:https://github.com/mozilla/fireplace/pull/1544
Flags: needinfo?(vidhan1995.jain)
(In reply to Vidhan Jain from comment #19)
> New PR link:https://github.com/mozilla/fireplace/pull/1544

Looks like it's almost there. Are you able to make the recommended change? Let us know.
Flags: needinfo?(vidhan1995.jain)
Assignee: vidhan1995.jain → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(vidhan1995.jain)
I would like to work on this bug
Looks like Vidhan is no more working on this bug, assigning this to Vaishali.
Assignee: nobody → vsham20
(In reply to Ram Dayal Vaishnav [:ramd] from comment #22)
> Looks like Vidhan is no more working on this bug, assigning this to Vaishali.

i am a newbie and this is my first bug, so can i get some help on how to start
Hi Vaishali,

We have documented few steps for newbies on wiki at https://wiki.mozilla.org/Marketplace/Contributing/Code which should be helpful for you. Let us know if you still have doubts :)
Whiteboard: [ktlo] [good first bug] → [good first bug]
I would like to work on this bug. This would be my good-first-bug.
Is this bug still open?! I guess it's resolved. Am I wrong? If not, I'm interested to work on this bug.
Flags: needinfo?(vaishnav.rd)
It is still open.
@David : I'm interested in working on this bug. I'll report the progress soon!

Thank You
Flags: needinfo?(vaishnav.rd)
David : Should the search box accept input of length zero and then return "Everything" returns X results
or shouldn't it get executed?

Thank You.
I would say that the search box should not accept input of length zero (so, strings that consist of only whitespace characters (space, tab, etc) should not be accepted). This is detailed in #c2.

Returning "Everything" (mentioned in #c6) is a sensible explanation of what searching on "" yields, but is probably confusing to the user.
Thanks for the information David. Now I'm clear about the task. Will work in it and report the progress soon.
David : Here's a patch for this bug https://github.com/mozilla/fireplace/pull/1618

I would feel glad if I can get feedback on the patch.

Thank You.
Just bumping this in case you missed the question in github.
Assignee: vsham20 → waseem.tabraze
Flags: needinfo?(waseem.tabraze)
Sorry for the delay David. I was in hospital since a month, So I couldn't go through the mails/github. I was just now looking at it.  Will work on it today and ping in you the changes soon. 

Thank You.
Flags: needinfo?(waseem.tabraze)
I want to work on this bug.
This is my first bug so how do i get started??
(In reply to Ayush Jain from comment #35)
> I want to work on this bug.
> This is my first bug so how do i get started??

There is a patch in flight for this. If it does not land, I'll let you know.
(In reply to David Durst [:ddurst] from comment #36)
> (In reply to Ayush Jain from comment #35)
> > I want to work on this bug.
> > This is my first bug so how do i get started??
> 
> There is a patch in flight for this. If it does not land, I'll let you know.

can you please explain what do you mean by flight??
Flags: needinfo?(ddurst)
(In reply to Ayush Jain from comment #37)
> (In reply to David Durst [:ddurst] from comment #36)
> > (In reply to Ayush Jain from comment #35)
> > > I want to work on this bug.
> > > This is my first bug so how do i get started??
> > 
> > There is a patch in flight for this. If it does not land, I'll let you know.
> 
> can you please explain what do you mean by flight??

Sorry, idiomatic expression.

I mean that there is currently an open Pull Request that would fix this issue, but if the author does not update it, then we're back to needing a patch for this. At that point, you could pick it up.
Flags: needinfo?(ddurst)
@ddurst : Made changes to the PR. :)
(In reply to Wasim Thabraze from comment #39)
> @ddurst : Made changes to the PR. :)

Merged! Thank you!
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.