keyword.enabled is half-broken, it's half enabled even when it's set to false

RESOLVED FIXED in Firefox -esr52



3 years ago
2 years ago


(Reporter: dqeswn, Assigned: KWierso, Mentored)



Firefox 55
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox52 wontfix, firefox-esr5253+ fixed, firefox53 fixed, firefox54 fixed, firefox55 fixed)


(Whiteboard: [fxsearch])


(1 attachment, 1 obsolete attachment)



3 years ago
When I add a single "word" of gibberish to the urlbar and press enter the expected thing happens: FF tries to resolve it, then I get an error page.

However when i add multiple words of gibberish and press enter a search with the default search engine is performed. Which shouldn't happen. I should get a "The address isn't valid" error.

It happens with a brand new profile after setting keyword.enabled=false


3 years ago
Summary: keyword.enabled is half-broken, it's half enabled even whet it's set to false → keyword.enabled is half-broken, it's half enabled even when it's set to false
Component: Search → Location Bar
Ever confirmed: true
Priority: -- → P3
Whiteboard: [fxsearch]
It is particularly annoying when using bookmark keywords regularly (because there will be spaces between the keyword and the arguments). If you mistype the keyword, then Firefox will leak the content of your address bar to Google, instead of displaying some "The address wasn't understood" local error page.

It is definitely new, although I'm not sure when it started. I occasionally mistype bookmark keywords, and it didn't send me to Google before. I don't think I changed anything to my configuration which could have changed this. I'm using Firefox 51 currently, and only noticed it yesterday.

I don't think anyone would want to disable "keyword.enabled", but still keep the feature when entering multiple words... Searches can often be a single word, so this would only work half the time... (note that including spaces before or after one word as a trick, does not send you to Google, even with "browser.fixup.alternate.enabled" disabled, because Firefox will trim the spaces, interpret the word as a top-level domain, and try to connect to it with HTTP).

Thus "keyword.enabled" being set to "false" should prevent Firefox from sending the content of the address bar to the default search engine, whether there are multiples words or not in the address bar. A "The address wasn't understood" error page should be displayed if no bookmark keywords are recognized.

The current workaround is to spend two hours and a half to:

1) Read the OpenSearch specs.

2) Create a custom OpenSearch plugin.

3) Set the template to something like "http://local/{searchTerms}" (and add " local" to your "/etc/hosts" file if not done before, so no DNS request is sent) because Firefox refuses to use only "{searchTerms}", or an empty string, or start with anything that's not some recognized protocols (can't use "about:blank", for example).

4) Then upload it somewhere because Firefox does not have a button to add OpenSearch plugins manually, nor accept local files, nor accept files in a "searchplugins" directory in your profile because it has been replaced by a binary file grouping all search engines.

5) Then create a webpage to point to the plugin because Firefox cannot load the plugin directly.

6) Then add the search bar to the toolbar because I don't use the search bar.

7) Then muck around some more, because "Firefox could not download the search plugin from: http://home.local/search.xml", without any detail in the browser console, isn't quite intuitive. Don't forget to also load your XML file in another tab, to reload it every time you change it, because caching. And to go to the preferences to remove the search engine every time you need to adjust something while testing, because you cannot see or edit them from the UI, and adding an engine with the same name results in an error, without the possibility to replace the current engine.

8) Then go to the preferences to set the new engine as the default one.

9) Then when you mistype a bookmark keyword, you just have to remove the URL prefix you added, and remove the first '+' which Firefox added to convert the spaces.

10) Then search Mozilla's bugzilla, and write a comment to give more details and a workaround, because the workaround is partial and annoying.

Note you cannot use "keyword.URL", because it has been remove some time ago. You couldn't set it to an empty string anyway, because it would then default to the default search engine.

Comment 2

2 years ago
Posted patch proposed fix (obsolete) — Splinter Review
This patch fixes the problem for me. It would be really good to fix this since it can lead to unintentional information leaks (small typos leak info to my default search engine).
Attachment #8835129 - Flags: review?(mak77)
Attachment #8835129 - Attachment is patch: true
Comment on attachment 8835129 [details] [diff] [review]
proposed fix

Review of attachment 8835129 [details] [diff] [review]:

This works, but it's not coherent with the rest, in particular, if we skip this last catch-all handler, we won't show any action, we should instead show a visit action.

You should rather modify _matchUnknownUrl to return a broken visituri entry if urifixup throws an NS_ERROR_MALFORMED_URI, something like:

    try {
      fixupInfo = Services.uriFixup.getFixupURIInfo(this._originalSearchString,
    } catch (e) {
=>    if (e.result == Cr.NS_ERROR_MALFORMED_URI && !Prefs.keywordEnabled) {
        let value = PlacesUtils.mozActionURI("visiturl", {
          url: this._originalSearchString,
          input: this._originalSearchString,
          comment: this._originalSearchString,
          style: "action visiturl",
          frecency: 0,

        return true;
      return false;

And then this needs a test, you can probably add one to toolkit/components/places/tests/unifiedcomplete/test_visit_url.js
See for example

After we have this, you can check the other tests are still good, through ./mach test toolkit/components/places/tests/unifiedcomplete/
Attachment #8835129 - Flags: review?(mak77)
Mentor: mak77

Comment 4

2 years ago
This is affecting me and others as well:

It would be nice to get the above fix out in the beta channel soon - it's a terrible and unexpected leak of private/confidential information.
Mark, do you plan to complete the work for your patch?
Flags: needinfo?(mcs)
Duplicate of this bug: 1346505
Keywords: privacy
Priority: P3 → P2

Comment 7

2 years ago
(In reply to Marco Bonardo [::mak] from comment #5)
> Mark, do you plan to complete the work for your patch?

I would like to, but I do not know when I will have time to work on this again. I think it is an important bug though and I really hope someone else finds time to work on it!
Flags: needinfo?(mcs)
Assignee: nobody → mak77

Comment 8

2 years ago
As a consequence of this information is also leaked to the "default" search engine when a search keyword is mis-typed or keypress is dropped.

So e.g. if the user has a search keyword 'w' for Wikipedia, and the letter 'w' is mis-typed or keypress doesn't register, the entire search goes to e.g. Google instead.


w Something that's none of Google's business -> Wikipedia

e Something that's none of Google's business -> Google

Lost keypress:
 Something that's none of Google's business -> Google

End result: Google aggregates mis-directed query with everything else it is inexorably collecting about the user. /tinfoil

Compounding this, the UI designers made it impossible to remove every 'standard' search engine (i.e. those in about:preferences#search), even if the user never wants to use search other than via keywords. As per Tsudojon's beautifully illustrated comment.

Also I'm fairly sure the keyword.enabled setting used to work properly (e.g. a year ago).
Priority: P2 → P1

Comment 10

2 years ago
Something like this? The test I added appears to pass, but I'm not sure if it's testing the right thing. 

When I do a two word query with this patch and keyword.enabled = false, it just brings up a "The address isn’t valid" error page, and the two word query stays in the address bar, unchanged.

Comment 11

2 years ago
FWIW, a local "Address isn't valid" error page/message is *exactly* what I want FF to bring up when I paste the wrong thing into the address bar and hit enter by reflex.

Comment 12

2 years ago
Comment on attachment 8848844 [details]
Bug 1318070 - Make sure multi-word queries are rejected when keyword.enabled is false

Attachment #8848844 - Flags: review?(mak77) → review+
Assignee: mak77 → wkocher


2 years ago
Attachment #8835129 - Attachment is obsolete: true

Comment 13

2 years ago
Pushed by
Make sure multi-word queries are rejected when keyword.enabled is false r=mak

Comment 14

2 years ago
Pushed by
Backed out changeset 13b38cd29a4a for esline failures a=backout
Make sure multi-word queries are rejected when keyword.enabled is false r=mak

Comment 15

2 years ago
Autoland's new zero tolerance policy means this got backed out in for an eslint failure.

Relanded with a fix in

Comment 16

2 years ago
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Comment 17

2 years ago
Comment on attachment 8848844 [details]
Bug 1318070 - Make sure multi-word queries are rejected when keyword.enabled is false

Would probably be good to uplift this as far as we're able to. I'll make sure the patch uplifts cleanly.

Approval Request Comment
[Feature/Bug causing the regression]:

[User impact if declined]:
Potential data leaks to the default search engine if you use bookmark keywords and mistype the keyword.

[Is this code covered by automated tests?]:
Test added in patch.

[Has the fix been verified in Nightly?]:
Not yet.

[Needs manual test from QE? If yes, steps to reproduce]: 
I don't think it does. But the steps to test are:
1. Flip keyword.enabled to false.
2. Type a two+ word query into the location bar and hit enter.
If it results in a search to yahoo (or whatever the default search engine is), that's a failure.
If it results in an internal error page and no search is performed, the patch is working as intended.

[List of other uplifts needed for the feature/fix]:

[Is the change risky?]:
Can't really speak to the risk, but it shouldn't be very risky.

[String changes made/needed]:

NOTE TO UPLIFTER: The commit that made it to mozilla-central is the one to be uplifted, otherwise you'll be uplifting an eslint failure.
Attachment #8848844 - Flags: approval-mozilla-beta?
Attachment #8848844 - Flags: approval-mozilla-aurora?

Comment 18

2 years ago
As far as I know, all supported versions except esr45 are affected.

Comment 19

2 years ago
Patch applies cleanly to aurora and beta (and release/esr52, if we want to go that far).

Comment 20

2 years ago
Comment on attachment 8848844 [details]
Bug 1318070 - Make sure multi-word queries are rejected when keyword.enabled is false

Would be nice to not leave this unfixed on esr52 for a year. I just applied the patch to esr52, built locally and checked that it works.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Could prevent a leak of things typed in the location bar to default search engines.
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See for more info.
Attachment #8848844 - Flags: approval-mozilla-esr52?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
Comment on attachment 8848844 [details]
Bug 1318070 - Make sure multi-word queries are rejected when keyword.enabled is false

Fix a broken multi-word queries issue when keyword.enabled is false. Aurora54+ & Beta53+.
Attachment #8848844 - Flags: approval-mozilla-beta?
Attachment #8848844 - Flags: approval-mozilla-beta+
Attachment #8848844 - Flags: approval-mozilla-aurora?
Attachment #8848844 - Flags: approval-mozilla-aurora+
Setting qe-verify- based on Wes' assessment on manual testing needs (see Comment 17) and the fact that this fix has automated coverage.

Gerry, we'll make sure to include this scenario when we run our recurring Awesomebar test suite on Beta 53 -- moving the ni? from Brindusa to myself.
Flags: qe-verify-
Flags: needinfo?(brindusa.tot)
Flags: needinfo?(andrei.vaida)
Comment on attachment 8848844 [details]
Bug 1318070 - Make sure multi-word queries are rejected when keyword.enabled is false

fix handling of keyword.enabled=false, esr52+
Attachment #8848844 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+

Comment 28

2 years ago
53.0b6 appears to fix the problem for me.  Thanks, all, for the rapid response.
Flags: needinfo?(andrei.vaida)
I have reproduced this bug with Nightly 53.0a1 (2016-11-16) (64-bit) on Ubuntu 16.04 LTS 64 Bit!

The fix's fix is verified with latest Beta 54.0b12 and latest Nightly 55.0a1 !

Build ID 	: 20170529095024
User Agent 	: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0

Build ID 	: 20170531100318
User Agent 	: Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
QA Whiteboard: [bugday-20170531]
I have reproduced this bug with Nightly 53.0a1 (2016-11-16) on Windows 10 LTS 64 Bit!

The bug's fix is verified with latest Beta 54.0b13 and latest Nightly 55.0a1 !

Build ID 	:20170601133324
User Agent 	:Mozilla/5.0 (Windows NT 10.0; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0

Build ID 	:20170603030204
User Agent 	:Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0
You need to log in before you can comment on or make changes to this bug.