Closed
Bug 336017
Opened 19 years ago
Closed 18 years ago
keyword.enabled doesn't work in Camino (1.8branch and trunk)
Categories
(Camino Graveyard :: Location Bar & Autocomplete, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
Camino1.5
People
(Reporter: mgelfo, Assigned: froodian)
Details
(Keywords: verified1.8.1)
Attachments
(1 file, 1 obsolete file)
3.75 KB,
patch
|
stuart.morgan+bugzilla
:
review+
mikepinkerton
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20060426 Camino/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20060426 Camino/1.0+
I updated to the nightly build and now keyword.enable doesn't work! I Get "This URL is not valid and cannot be loaded"
Reproducible: Always
Steps to Reproduce:
1. Set keyword.enable to true in about:config
2. Go to location bar and type search string, e.g. "britney spears"
3. Cry because you don't get Britney Spears listings on Google
Actual Results:
"This URL is not valid and cannot be loaded"
Expected Results:
Keyword you typed should be sent to the search engine specified in keyword.URL
Comment 1•19 years ago
|
||
Does it work in the corresponding Firefox build? My guess would be not, since Camino doesn't do anything specific to support the pref.
If you type a single-word phrase, you get the normal fixup behavior, but I can confrim this is broken on the trunk.
The root cause seems to be bug 331522, which changed the behavior, disabled keyword.enabled entirely for all apps, and then enabled it for Fx. We depend on the xpfe/suite hookup, bug 332668, to get it working again.
Status: UNCONFIRMED → NEW
Depends on: 332668
Ever confirmed: true
Summary: keyword.enable doesn't work → keyword.enabled doesn't work in Camino (1.8branch and trunk)
Comment 3•19 years ago
|
||
(In reply to comment #2)
>We depend on the xpfe/suite hookup, bug 332668, to get it working again.
I don't think you do, because you're an embedder. Instead you probably need to call nsIWebNavigation::LoadURI with the allow third party fixup flag.
(In reply to comment #3)
> I don't think you do, because you're an embedder. Instead you probably need to
> call nsIWebNavigation::LoadURI with the allow third party fixup flag.
That comes up with no hits for Camino in LXR, either (Stuart, what did you search for last night when you said it seemed like we were dependent?)...
Keywords: regression
Target Milestone: --- → Camino1.1
Comment 5•19 years ago
|
||
I was looking for keyword.enable; Camino never reads that value. So before the changes Camino didn't have to explicitly support it, but now it does?
Smokey, Camino calls LoadURI here:
http://lxr.mozilla.org/mozilla/source/camino/src/embedding/CHBrowserView.mm#422
No longer depends on: 332668
Whiteboard: [good first bug]
Comment 6•19 years ago
|
||
Now it has to explicitly force LoadURI to support it.
Assignee | ||
Comment 7•19 years ago
|
||
This puts the support for the third party fixup flag in CHBrowserView, but
if (flags & NSLoadFlagsAllowThirdPartyFixup)
is never true (regardless of the pref's state). Is this not working, or am I merely overlooking something obvious?
Assignee: nobody → stridey
Status: NEW → ASSIGNED
Comment 8•19 years ago
|
||
Those code blocks in loadURI:... are just wrappers to make the call sites in the Camino code not need to know about core contant values. Adding that code will just make it so that if loadURI:... were explicitly called with NSLoadFlagsAllowThirdPartyFixup it would be passed along--but of course nothing is calling it that way since it didn't exist.
If I'm understanding neil correctly, what you want is:
if (<keyword.enable is on>)
navFlags |= nsIWebNavigation::LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
(And there would be no need to wrap the constant for Camino use.)
There's a larger question of whether Camino wants to take advantage of the new ability to selectively disable fixup where it doesn't make sense to do, but since it's hidden and disabled by default, just getting the old behavior back is good enough for now; evaluating each call to loadURI:... isn't worth the trouble.
Assignee | ||
Comment 9•19 years ago
|
||
Per Pink and IRC, we should be doing this at a higher level than CHBrowserView.
Attachment #226039 -
Attachment is obsolete: true
Attachment #227140 -
Flags: review?(stuart.morgan)
Comment 10•19 years ago
|
||
Comment on attachment 227140 [details] [diff] [review]
Patch
r=me
Attachment #227140 -
Flags: superreview?(mikepinkerton)
Attachment #227140 -
Flags: review?(stuart.morgan)
Attachment #227140 -
Flags: review+
Comment 11•18 years ago
|
||
Comment on attachment 227140 [details] [diff] [review]
Patch
sr=pink
Attachment #227140 -
Flags: superreview?(mikepinkerton) → superreview+
Assignee | ||
Updated•18 years ago
|
Whiteboard: [good first bug] → [good first bug] [needs checkin]
Comment 12•18 years ago
|
||
Fixed trunk and branch.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: regression → fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [good first bug] [needs checkin]
Comment 13•18 years ago
|
||
V/fixed.
works in camino 1.5.
haven' tested the trunk, but did check lxr.
You need to log in
before you can comment on or make changes to this bug.
Description
•