Closed Bug 1177895 Opened 9 years ago Closed 6 years ago

Restriction tokens not removed from "Search with" default action

Categories

(Firefox :: Address Bar, defect, P3)

Product:
Firefox
Component:
Address Bar
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 65

People

(Reporter: mak, Assigned: mak)

References

Details

(Whiteboard: [unifiedautocomplete][fxsearch])

Attachments

(1 file)

Capture.PNG
(deleted), image/png
Details
STR: 1. have a tab open, for example mozilla.org 2. type "% moz" in the locationbar to restrict the results to open tabs 3. Note the first entry is "% moz - Search With Engine" 4. Note we do indeed search for "% moz" instead of just "moz"
Flags: firefox-backlog+
Blocks: UnifiedComplete
Rank: 35
Priority: P2 → P3
this sucks but it's not a feature blocker.
No longer blocks: UnifiedComplete
Assignee: nobody → adw
Status: NEW → ASSIGNED
This is more complex than it seems at first. When you type "% foo" and hit return, the input's value is simply "% foo". When you type "% foo", press down, and then press up to select the heuristic result again, now the input's value is a moz-action of type searchengine. So first, it seems like the input's value should be the same in both cases since the heuristic result is selected in both cases. As it is, we take different code paths. Second, fixing the second case is simple (modify UnifiedComplete.js), but what should we do in the first case? Which leads to... Third, what if I actually want to search for "% foo", including the "%"? I guess we already answered that question a long time ago when we added restriction tokens though? "You can't do that." Plus, this error happens when the input is "% foo": JavaScript error: chrome://browser/content/urlbarBindings.xml, line 877: URIError: malformed URI sequence It happens here for the `input` key because it ends up as "%%20 foo": http://mxr.mozilla.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#877 I'd really like to consolidate all the moz-action code to one place, parsing and creating moz-actions, to avoid problems like that. I have a patch that does that in PlacesUtils, but I guess maybe it shouldn't be part of this bug.
(In reply to Drew Willcoxon :adw from comment #2) > This is more complex than it seems at first. When you type "% foo" and hit > return, the input's value is simply "% foo". When you type "% foo", press > down, and then press up to select the heuristic result again, now the > input's value is a moz-action of type searchengine. I guess we may add the tokens or the original search string to the moz-action payload. > Third, what if I actually want to search for "% foo", including the "%"? I > guess we already answered that question a long time ago when we added > restriction tokens though? "You can't do that." Yeah, restriction tokens are nice, but totally undiscoverable and cause this problem. It would be great to have an alternative but honestly I'm not sure what it may be. Maybe restriction tokens should be a little bit longer, having an activation char that is unlikely someone would search (something like §) and a small descriptor (§b - bookmark, §h - history, §t - tabs, ...), this wouldn't help discoverability but it would help supporting special chars. > I'd really like to consolidate all the moz-action code to one place, parsing > and creating moz-actions, to avoid problems like that. I have a patch that > does that in PlacesUtils, but I guess maybe it shouldn't be part of this bug. bug 1054816?
(In reply to Marco Bonardo [::mak] from comment #3) > I guess we may add the tokens or the original search string to the > moz-action payload. also, actually each richlistitem already has the original search string in an attribute, iirc.
Blocks: 1315509
(In reply to Drew Willcoxon :adw from comment #2) > This is more complex than it seems at first. When you type "% foo" and hit > return, the input's value is simply "% foo". When you type "% foo", press > down, and then press up to select the heuristic result again, now the > input's value is a moz-action of type searchengine. This is no more true in FF 51 and 52, so this should be more easily fixable just in Unifiedcomplete.js > Third, what if I actually want to search for "% foo", including the "%"? I > guess we already answered that question a long time ago when we added > restriction tokens though? "You can't do that." We could use the override action modifiers.
Blocks: 1334019
I think we should also support cases like "%something" where the modifier comes just before a word. I think it's not wise to do the same in case it comes after it, since "50%" for example is a pretty much valid value to search for. This would basically also solve bug 1334019. Re: Searching for "% foo", while we could support overrides, as I said in comment 5, I feel like that adds too much hidden complication, so for now I'd just stick to "not possible".
Blocks: 1386548
Fixed by bug 1315509
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Assignee: adw → mak77
No longer blocks: 1315509
I have reproduced this bug with Nightly 41.0a1 (2015-06-27) on Windows 7, 64 Bit! This bug's fix is verified with latest Beta! Build ID 20181220174318 User Agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0
QA Whiteboard: [bugday-20181219]
Flags: qe-verify+
Attached image Capture.PNG (deleted) —

Hello,

I'm trying to verify that this bug is resolved, but between the Nightly build from 2015-06-26 and the current Beta 65.0b10 Build ID: 20190110221328 I can't see any difference when following the STR. I attached a screenshot of the current beta and as you can see when I type "% redd" the first suggestion is still "search with engine".

Am I missing something?

Flags: needinfo?(mak77)

(In reply to Daniel Cicas [:dcicas], Release QA from comment #11)

I'm trying to verify that this bug is resolved, but between the Nightly build from 2015-06-26 and the current Beta 65.0b10 Build ID: 20190110221328 I can't see any difference when following the STR.

Why a so old build? I understand the will to reproduce the original issue on the original version, but I honestly don't remember how the address bar was behaving 4 years ago, It's possible the original bug was filed off a project branch or a future patch, also restriction characters changed recently. Any version from the last year should reproduce this bug trivially, using either % or ? as a special char.

In that version, does confirming the first entry actually search for "redd" or for "% redd" on Google?

Flags: needinfo?(mak77)

So I tested the following builds:

  • FX 62.0a1 Build ID 20180528220216.

When you search for the "% redd" in the location bar, the first entry is "% redd - search with engine". After confirming the search, the first entry is "% redd" on Google.

  • FX 65.0b10 Build ID 20190110221328.

When you search for the "% redd" in the location bar, the first entry is "% redd - search with engine". After confirming the search, the first entry is "redd" on Google.

Additional note:
Using "?" as a special char.the first entry on Google is "redd"

(In reply to Daniel Cicas [:dcicas], Release QA from comment #13)

  • FX 65.0b10 Build ID 20190110221328.

When you search for the "% redd" in the location bar, the first entry is "% redd - search with engine". After confirming the search, the first entry is "redd" on Google.

This is not what I see, I just tried with 65.0b10, the first entry for me is "redd - searach with google".

And if so, it's indeed the expected behavior, the % should not appear in the first address bar entry, nor in the search engine searched string.

Hello,

I have verified this issue as fixed on Fx Beta 65.0b12 Build ID:20190117232427 on Windows 10 x64, Ubuntu 18.04.1 LTS and macOS X 10.13.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1315509
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: