Closed Bug 143080 Opened 23 years ago Closed 22 years ago

Keyword lookup regression

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0.1

People

(Reporter: adamlock, Assigned: adamlock)

References

(
URL
)

Details

(Keywords: intl, regression, Whiteboard: [adt1 RTM] [Needs a= ],custrtm-)

Attachments

(1 file, 1 obsolete file)

Patch (try again)
2.20 KB, patch
jud
: review+
rpotts
: superreview+
jesup
: approval+
Details | Diff | Splinter Review 
Typing a word such as "webmail" into the location bar should go off to the
keyword lookup if Internet Keywords are enabled.

Currently you have to type ?webmail to force this behaviour.
The checkin for bug 109309 removed the keyword URL creation code from
nsWebShell.cpp, making it use the keyword fixup code in the fixup object. That
implementation only assumes a string qualifies for keyword fixup if it passes a
few tests including containing one space or more.

The solution is to make keyword fixup a more explicit request during the call to
mURIFixup->CreateFixupURI in nsWebShell, possibly using a new flag.
And the most important thing for us: because Asian languages never have single 
byte space, the whole Internet Keyword is not working now. This could be our JA 
beta showstopper, please help to fix asap. Thanks.
See more information in bugscape 15097.
I think this is broken in general, not just for Japanese beta. For example, the
"List of keywords" page tell me I can type city name. If you type "boston" now,
it will redirect to http://www.boston.com now.
Attached patch Patch (obsolete) — — Splinter Review 
This patch reinstates the old piece of code. I haven't touched it much except
for changes in the string classes and to fit in with the rest of this method.
Other patch contained some other diffs. This one contains just the reinstated
code.
Attachment #82829 - Attachment is obsolete: true
Adam, can you have the patch reviewed asap? So we can have the fix in bera.
Keywords: nsbeta1
Regression! 


------- Additional Comment #11 From Naoki Hotta 2002-05-08 11:48 -------

>regression: Internet Keyword does not work for double byte
Actually not work for ASCII either. The bugzilla bug has to be a generic one
instead of i18n specific.
Keywords: regression
jaime, this could be a stopper for ja & zh builds.
This looks like a dupe or would be related to bug 15097, which ylong reported
yesterday. If it is the same bug, or related, PdM has already decided that bug
15097 was not a stopper for beta.
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt1 RTM]
doh! i meant bugscape bug 15097.
Can someone test the patch with international keywords? If it works please
confirm it here so I can get the r=
ylong/ftang - can you help adam out with this patch? thanks!
I agree that this is not a beta stopper for ja or zh. Let's make sure it gets
fixed for RTM. 
I did a brief test on nhotta's machine with the patch, the Japanese keywords(as
well as US) works fine.
Blocks: 143047
Blocks: 141008
Keywords: intl
Can I have an r= from someone on this list? Thanks.
do we really want to send http://foo to the keyword server?

it seems like we only want to apply the 'www' and 'com' rewriting if a
http/https scheme is present...

-- rick

If you type "foo" into the address bar then Mozilla fixes it up into http://foo/
and tries to load that. It is only when this fails that the keyword lookup is
invoked.

So yes, we do want to call the keyword server when http://foo/ fails.
Comment on attachment 82831 [details] [diff] [review]
Patch (try again)

sr=rpotts@netscape.com
Attachment #82831 - Flags: superreview+
Attachment #82831 - Flags: review+
valeskimark the patch as has-reviewed in 2002-05-16 10:05:17 
adamlock- please land into trunk asap and mark this bug as fixed so QA will
verified the trunk .thanks
Fix checked into trunk
Seeking adt approval. Patch is self contained and a prominent fix.
Keywords: adt1.0.0
Adam - I see that you are listed as QA for this bug. Who besides you can verify
this one as fixed. We'd like to see QA verify this one, before giving it the
ole' ADT+. thanks!
Keywords: approval
Whiteboard: [adt1 RTM] → [adt1 RTM] [Needs a= & ADT+]
Nominate Frank
QA Contact: adamlock → ftang
Andreas, can you have someone try this out?
QA Contact: ftang → andreasb
I checked it on 05-17 trunk build/Win-ME-JA:
1. The one work kepword is not longer try to go xxxx.com, however, if I type
"webmail", I won't get the netscape webmail page, but a search result for "webmail".
2. I can not test the Japanese keyword in this US build, all I got was not found
result.

I remember when I tested this on nhotta's machine with the fix patch applied on
a mozilla build, both cases above worked. 

You must ensure "Enable Internet Keywords" is checked in
Preferences|Navigator|Smart Browsing
Yes, it was checked.
In comment #26:
> 2. I can not test the Japanese keyword in this US build, all I got was not found
result.

Actually it was not true - when I set the accept language to "ja", the Japanese
keywords work.
mark this bug as FIXED since it land into the trunk already
IQA- if the bug is verified , please mark it as verified.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
One thing I want to confirm before I mark it as verified is:
by type "webmail" in URL bar will get search result instead of Netscape webmail
page.
If it is OK, then I'll mark it as verified.  Otherwise, please fix it. 

Reopening. It's fixed in the trunk but I you want it in the branch too right?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
>Reopening. It's fixed in the trunk but I you want it in the branch too right?
I don't think drivers will give you a= for branch and adt give you adt1.0.0+
unless you mark it fixed and verified on the trunk 
Andreas - Can you tema verify this on the trunk?

Adam - If this is checked into the trunk, pls resolve as fixed, so QA can verify
the fix. [Note: When this is fixed on the branch, the convention is to add
fixed1.0.0].
Òk, marking FIXED again, please verify.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Like I said in comment #31, the webmail don't go to the Netscape web mail page,
but a search result.
Is it OK?
The thing with not appending .mcom.com onto the end of local names is something
else, not to do with keyword fixup (or the fixup object in general). This patch
is specifically to cause unresolved URLs to be sent to the keyword lookup service.
In that case, I'm marking this as verified.
Status: RESOLVED → VERIFIED
adding adt1.0.0+ for checkin to the 1.0 branch.  Please get driver approval
before checking in.
Keywords: adt1.0.0 → adt1.0.0+
Whiteboard: [adt1 RTM] [Needs a= & ADT+] → [adt1 RTM] [Needs a= ]
Reopening. Drivers don't want this for 1.0.0 but Brendan gives an a= for 1.0.1
which means branch checkin after 1.0 ships (hopefully a week or so).
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Target Milestone: --- → mozilla1.0.1
changing to adt1.0.1+ for checkin to the 1.0 branch for the Mozilla1.0.1
milestone.  Please get drivers approval before checking in.
Keywords: adt1.0.0+ → adt1.0.1+
Comment on attachment 82831 [details] [diff] [review]
Patch (try again)

please check into the 1.0.1 branch ASAP. once landed remove the 
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword
Attachment #82831 - Flags: approval+
Whiteboard: [adt1 RTM] [Needs a= ] → [adt1 RTM] [Needs a= ],custrtm-
Fix checked
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Keywords: mozilla1.0.1+ → fixed1.0.1
Resolution: --- → FIXED
Blocks: 146292
No longer blocks: 141008
Verified fixed on 06-05 branch build.
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.1 → verified1.0.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: