1075069 - Wrong search terms are highlighted on Google

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

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

STR:
Set Google as the default search engine
In the address bar, type "test 123" and hit Enter
Wait for the Google search results page to load
Look at the address bar and notice that `test+123` are highlighted
Change the query to "test 345" and hit Enter
Wait for the new search results to load
Look at the address bar

Expected:
`test+345` are highlighted

Actual:
`test+123` are highlighted

The old `test+123` are still in the URL (the first instance of q=), and the new `test+345` are at the end of the URL (the second instance of q=)

Subsequent searches change the second instance, but always keep the first instance unchanged.

Comment 1

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

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #0)
> Change the query to "test 345" and hit Enter

The key here is that the query must be changed from within the search results page using the Google search box in content.

Comment 2

[Dão Gottwald [:dao]]

Bummer. Even if we highlighted the right search terms here, users might not see them since they're at the end of the URL and likely cut off, leading to inconsistent behavior. Given this and bug 1069903 I tend to think that bug 1047469 should be backed out...

Comment 3

[Dão Gottwald [:dao]]

[Tracking Requested - why for this release]: because bug 1047469 is riding this train

Comment 4

[Marco Bonardo [:mak]]

So, the hilight was a mockup from Boriss, but I'm sure we have another bug with a different UI mockup from phlsa that rather hides completely the search url (until you want to edit it), showing something like "Google - search terms".
That could use the same search service backend as simpel hilight.
I just have to find it...

Comment 5

[Marco Bonardo [:mak]]

found it! bug 1048223.
I think it's very hard to fix this bug with the current approach, while that approach would allow us to handle this case.

Comment 6

[Dão Gottwald [:dao]]

Attachment: backout.diff

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8498347 [details] [diff] [review]
backout.diff

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

Too bad this has to be so complicated :/

Comment 8

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/fdcb83cd2139

Comment 9

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

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

Comment 10

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

Closing this bug as bug 1048223 will track the new implementation.

Comment 11

[Andrei Vaida [:avaida]]

Since Bug 1047469 was marked as WONTFIX and this implementation will be tracked in Bug 1048223, I believe that at this point there's nothing here for manual QA to poke at. Marking this issue as qe-verify-.

Dao, if you think we should look at this bug differently, please feel free to flip back the flag.