1294733 - Awesomebar search buttons show auto completed text not user input

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[alex_mayorga]

¡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

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

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?

Comment 2

[Alice0775 White]

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

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

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.

Comment 4

[Alice0775 White]

Attachment: screenshot

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

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?

Comment 6

[Alice0775 White]

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

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

OK, thanks for clarifying.

Comment 8

[Drew Willcoxon :adw]

You're right, thanks for filing.  I think this should be easy to fix.

Comment 9

[Drew Willcoxon :adw]

Attachment: Bug 1294733 - Awesomebar search buttons show auto completed text not user input.

Review commit: https://reviewboard.mozilla.org/r/71388/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71388/

Comment 10

[Drew Willcoxon :adw]

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 11

[Florian Quèze [:florian]]

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.

Comment 12

[Drew Willcoxon :adw]

Comment on attachment 8780749 [details]
Bug 1294733 - Awesomebar search buttons show auto completed text not user input.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/71388/diff/1-2/

Comment 13

[Drew Willcoxon :adw]

Yeah, that should be OK, thanks.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e79a975ce7b7
Awesomebar search buttons show auto completed text not user input. r=florian

Comment 15

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

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

Comment 16

[Azmina]

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]

Comment 17

[alex_mayorga]

¡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