Closed Bug 1529642 Opened 9 months ago Closed 9 months ago

PBM handoff shows history suggestions with @keywords (due to suggestions being disabled?)

Categories

(Firefox :: Search, defect)

66 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 67
Tracking Status
firefox65 --- wontfix
firefox66 + verified
firefox67 --- verified

People

(Reporter: mconnor, Assigned: adw)

References

Details

Attachments

(2 files)

[Tracking Requested - why for this release]: Unexpected UX in a new feature.

STR:

  1. In current Nightly, type @google in a regular window, then type some common terms. Only search suggestions should be shown.
  2. Open a Private Browsing window and repeat the process, typing terms that match browser history. You will see history items despite the search prefix.

Where there's an @keyword, we should never show history.

This looks like a bug in UnifiedComplete. After we fetch all search suggestions, we properly check to see whether an @ alias was used, and if so, we stop adding more results [1]. But if we skip fetching suggestions in the first place -- which we do in private browsing mode -- we also skip the @ alias check and therefore continue to add other types of results, like history results.

[1] https://searchfox.org/mozilla-central/rev/b36e97fc776635655e84f2048ff59f38fa8a4626/toolkit/components/places/UnifiedComplete.jsm#972

Bug 1496814 is where this was implemented

Blocks: 1496814
Assignee: nobody → adw
Status: NEW → ASSIGNED

We already have a check for stopping the search when using an @ alais or restricting to suggestions. Move the check outside the block where we add suggestions so that it's used regardless of whether we added suggestions. Update the existing test to run in a private context.

Mike, just to clarify some related behavior, should we be showing search suggestions in pbm? We don't currently and haven't ever afaik, but we just wanted to be clear on that. That means that when this bug is fixed, when you type @google in pbm, all you'll see is the pre-highlighted "Search with Google" result.

Flags: needinfo?(mconnor)

Yes, that's correct. We may at some point consider an opt-in for suggestions in PBM, but that's not in scope for this bug/release.

Flags: needinfo?(mconnor)
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5ad94d06e585
When using a search engine @ alias, the only results that should be shown are search suggestion results, including in private contexts r=mak
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Flags: qe-verify+

Would you like to request uplift to beta? We have 3 beta builds left before RC week.

Flags: needinfo?(adw)

I can't speak for Mike, but I don't see a reason not to request uplift, so I'll do that (and leave the needinfo on me until I do).

Attached patch Beta patchSplinter Review

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Search keywords, bug 1496814
  • User impact if declined: Users will incorrectly see history results when using search keywords in private windows.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Please see comment 0
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is covered by an automated test, which I ran on beta and passes. I manually tested it too. It's a small change.
  • String changes made/needed: None
Flags: needinfo?(adw)
Attachment #9047246 - Flags: approval-mozilla-beta?
Flags: in-testsuite+
Comment on attachment 9047246 [details] [diff] [review]
Beta patch

Fix for search suggestions in private browsing. 
OK for uplift for beta 12.
Attachment #9047246 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [qa-triaged]

Verified - fixed on latest Nightly 67.0a1 (2019-02-27) (64-bit) on Windows 10, Mac OS 10.13 and Ubuntu 16.04.
Only the search suggestion will be shown highlighted in PBM.
Will verify it on Beta too when the patch will be landed.

Verified fixed on latest Firefox Beta 66.0b12 (64-bit)(Build ID: 20190301012442) on Windows 10, MacOS 10.13 and Ubuntu 16.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.