Prefixing text in the location bar with "?" includes that in the search

RESOLVED FIXED in Firefox 4.0b8

Status

()

Firefox
Location Bar
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: mossop, Assigned: Gavin)

Tracking

({regression})

Trunk
Firefox 4.0b8
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
It used to be the case that putting a ? before what you typed into the location bar forced a search of the following test using keyword.URL however the ? was not included in the search. Now it is getting included.
(Reporter)

Comment 1

7 years ago
This is irritating when you need to use this to force a search for a string that normally would resolve to a website.
blocking2.0: --- → ?
Looks broken. Blocking+. Anyone know what could've caused this (and the tests still passing)?
blocking2.0: ? → final+
Keywords: regressionwindow-wanted
OS: Mac OS X → All
Hardware: x86 → All
(In reply to comment #2)
> Looks broken. Blocking+. Anyone know what could've caused this (and the tests
> still passing)?
Not offhand.  I don't recall anything changing with the keyword code recently...

Comment 4

7 years ago
keyword.URL was changed when disabling browse by name.

Just checking though, "?" wasn't a bookmark keyword or a search engine keyword? It was some special hardcoded token?
(Reporter)

Comment 5

7 years ago
Yes it was hardcoded in the docshell fixup stuff somewhere.

Comment 6

7 years ago
Oh I see. Seems like the code is still there:

http://hg.mozilla.org/mozilla-central/file/29ea30892eae/docshell/base/nsDefaultURIFixup.cpp#l357

Comment 7

7 years ago
cc'ing gavin, since he made the docshell changes for disabling browse-by-name.

Comment 8

7 years ago
One thing that changed is that it doesn't seem to be hitting that code I linked in comment 6. That code appends the keyword whereas trunk seems to be inserting it in the middle:

var {classes:Cc,interfaces:Ci}=Components; Components.classes["@mozilla.org/docshell/urifixup;1"].getService(Ci.nsIURIFixup).createFixupURI("? foo", 1).spec

http://www.google.com/search?q=%3F+foo&ie=utf-8&...

Updated

7 years ago
Depends on: 586821
Keywords: regressionwindow-wanted

Comment 9

7 years ago
Oh hehe. keyword.URL is empty ;)

Mossop: If you set this, do you get the original behavior?

keyword.URL;http://www.google.com/search?ie=UTF-8&oe=UTF-8&sourceid=navclient&q=
(Reporter)

Comment 10

7 years ago
Yeah that'd be the problem

Comment 11

7 years ago
(In reply to comment #9)
> Oh hehe. keyword.URL is empty ;)

keyword.URL was intentionally changed to empty, since we want the dynamically set affiliate bits in the URL.
(Reporter)

Comment 12

7 years ago
(In reply to comment #11)
> (In reply to comment #9)
> > Oh hehe. keyword.URL is empty ;)
> 
> keyword.URL was intentionally changed to empty, since we want the dynamically
> set affiliate bits in the URL.

Right, but as it stands since bug 586821 if there is no keyword.URL then MangleKeywordIntoURI is never called and so the special handling of the ? prefix doesn't happen.
Assignee: nobody → gavin.sharp
Created attachment 487258 [details] [diff] [review]
patch

The main change is to just always do the "?"-removal and space-trimming, regardless of which path is taken (search service or pref value).

Also updated browser_keywordSearch to mostly cover nsDefaultURIFixup::KeywordToURI (it's under browser because it depends on Firefox-specific behavior details - don't really want to do the work to remove that dependency).

Aside: the test was kind of busted (used nsIWebProgressListener.NOTIFY_STATE_DOCUMENT which doesn't exist, which didn't matter because of bug 608628), but that was only exposed when I made it do more than one load, so I had to fix that too.
Attachment #487258 - Flags: review?(bzbarsky)
Comment on attachment 487258 [details] [diff] [review]
patch

r=me
Attachment #487258 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/4c55c33b49d3
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
You need to log in before you can comment on or make changes to this bug.