Closed Bug 1294733 Opened 3 years ago Closed 3 years ago

Awesomebar search buttons show auto completed text not user input

Categories

(Firefox :: Address Bar, defect, P2)

51 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 51
Tracking Status
firefox51 --- verified

People

(Reporter: alex_mayorga, Assigned: adw)

References

Details

(Whiteboard: [fxsearch])

Attachments

(2 files)

¡Hola!

Steps:
Type "test" in the URL bar, it autocompletes here to "testpilot.firefox.com" since I've visited that page.

Expected result:
The search buttons at the bottom of the suggestions search for the actual "test" input.

Actual result:
The search buttons at the bottom of the suggestions search for the auto completed string "testpilot.firefox.com" not the actual "test" input.

¡Gracias!
Alex
Blocks: 1180944
I can't reproduce what you are seeing. If I type "test" and nothing else and then click on one of the search engine buttons the search term is still "test". Can you provide more detailed steps or a screencast of what you are seeing?
(In reply to Panos Astithas [:past] from comment #1)
> I can't reproduce what you are seeing. If I type "test" and nothing else and
> then click on one of the search engine buttons the search term is still
> "test". Can you provide more detailed steps or a screencast of what you are
> seeing?

I can reproduce the problem if you had been bookmark or visitted this Bugzilla before.

Try to search term "bug" (without quotations) from location bar, Nightly will go to bugzilla.mozilla.org instead search "bug".
Nope, it still searches "bug" for me regardless of how I initiate the search (mouse click or tab key). I'm on Mac though, I haven't tested on Windows yet.
Attached image screenshot
Oh, I see that the label suggests that the search will be about the wrong thing, but clicking on it actually searches for the term. There is definitely a bug in the string we display in that label. Can you confirm that this is what you get if you actually click to search?
> There is definitely a bug in the string we display in that label.

Yep, visual indication problem of UI. There is no problem in actual search function.

> Can you confirm that this is what you get if you actually click to search?

I get search results of "the user typed text" with the search engine as expected.
OK, thanks for clarifying.
OS: Windows 7 → All
Priority: -- → P2
Hardware: x86_64 → All
Whiteboard: [fxsearch]
You're right, thanks for filing.  I think this should be easy to fix.
Assignee: nobody → adw
Status: NEW → ASSIGNED
This is a different facet of the recent bug we fixed where we'd search for the autocompleted value instead of the typed value.  So this fix uses the same approach basically.
Comment on attachment 8780749 [details]
Bug 1294733 - Awesomebar search buttons show auto completed text not user input.

https://reviewboard.mozilla.org/r/71388/#review68942

Looks good to me, with one minor comment.

::: browser/components/search/content/search.xml:1285
(Diff revision 1)
> -              // The urlbar's value property can be a moz-action URI, but we
> -              // want the value that the user sees, which is textValue.  So see
> -              // if the textbox has a textValue property, and use it if so.
> -              this.query = typeof(event.target.textValue) == "string" ?
> -                           event.target.textValue :
> +              // Allow the consumer's input to override its value property with
> +              // a oneOffSearchQuery property.  That way if the value is not
> +              // actually what the user typed (e.g., it's autofilled, or it's a
> +              // mozaction URI), the consumer has some way of providing it.
> +              this.query = typeof(event.target.oneOffSearchQuery) == "string" ?
> +                           event.target.oneOffSearchQuery :

Looks to me like this could be simplified to:
this.query = event.target.oneOffSearchQuery ||
             event.target.value;

The || operator won't cause a warning if it's undefined, and it will avoid calling the oneOffSearchQuery getter twice. My suggestion will cause a problem if "" is a valid value though, but I don't think we can autocomplete anything if the user hasn't typed yet; is that correct?
If we need to handle the "" value, please use a temporary variable to avoid the double call to the getter.
Attachment #8780749 - Flags: review?(florian) → review+
Yeah, that should be OK, thanks.
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e79a975ce7b7
Awesomebar search buttons show auto completed text not user input. r=florian
https://hg.mozilla.org/mozilla-central/rev/e79a975ce7b7
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
I have reproduced this bug with Nightly 51.0a1 (2016-08-12) on Windows 8.1 , 64 bit!

This Bug's fix is verified on Latest Nightly 51.0a1.

Build ID : 20160816030459
User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0

[bugday-20160817]
¡Hola Drew!

This now works as expected on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0 ID:20160822064441 CSet: 194fe275b4e60ded2af6b25173eec421f0dba8ad

¡Gracias!
Alex
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.