11083 - URL bar assumes english keyboard input

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P3)

Description

[tague]

The URL bar is assuming that it will be getting English input and throwing up
assertions when non-English data is typed.  [It actually displays, but throws up
erronious asserts]

steps to reproduce :
(1) turn on Japanese IME
(2) type some Japanese in URL bar
(3) **** watch asserts on the console

With internet keywords and other similar idenitifiers, it's quite possible that
someone could type Japanese into the URL bar.

Comment 1

[don]

Radha, do we fix this or does the i18n team fix it?

Comment 2

[Radha on family leave (not reading bugmail)]

I need some enlightment from the widget gurus and the I18N folks. cc'ing them.

Comment 3

[Radha on family leave (not reading bugmail)]

Reassign to ftang. I think he knows better about the status of this.

Comment 4

[Frank Tang]

Dear tague, is this still happening ? if it throw assert, please attach the
stack trace so they can figure out who should own it. Please use CVS blame to
see who put in that assert and reassing the bug to him/her. Thanks.

Comment 5

[leger]

Updating QA Contact.

Comment 6

[Frank Tang]

tague is doomed, reassign this to myself.

Comment 7

[Frank Tang]

Mark it as M11 assigned. Add akkana, buster to the cc list.

Comment 8

[Frank Tang]

The problem is not input, the problem is some event handler change the content
of the content of the URL bar after we hit RETURN
It seem that we should check BrowerLoadURL() in
mozilla/xpfe/browser/resoures/content/navigator.js

We need to check
1. the return value of document.getElementById('urlbar')
2. the return value of bmks.FindShortcut
3. appCore.loadUrl

according to xpfe/browser/resources/content/navigator.xul
the urlbar have the following attribute-
id=urlbar
type=text
chromeclass=location
onkeyup="if (event.which == 13) { BrowserLoadURL(); }"

add rjc/waterson to the cc list since he own bmks.FindShort. rjc/waterson, I
don't think you need to do anything for this bug right now. But you probably
should aware this issue if you need to add more code (URL shortcut ???) to the
BrowserLoadURL.

Comment 9

[Frank Tang]

a seperate but related issue. I think the
onkeyup="if (event.which == 13) XXX " is bad. The reason is we use RETURN key to
commit IME to commit IME, in that case, the user may want to type MORE Japanese.
Maybe we should change the KEY up / down to other value when the RETURN is for
commit IME.

Comment 10

[Frank Tang]

add radha/law and editors folks to the cc list.

Comment 11

[Frank Tang]

I think my concern about the onkeyup have been addressed in spec level by
kathy's posting of  "key event handling revision 3"
news://news.mozilla.org/37F16680.8C3C7017%40netscape.com

>2. Key Event Generation

>Native input methods (used when typing Japanese), aka IME, must have first
>crack at all keyboard events this includes
>accelerators and keybindings because they might be used by the native input
>method (such as to turn them on and off).
>While the IME is active, key events (keydown, keypress, keyup) will not be
>generated.  IME input generates text events
>which are handled by a separate set of handlers.  If a key event isn't consumed
>by the native input method, it will result in a
>sequence as described below.

tague- is that true that we will do this ? filtering out keyup/down/press ? If
so, I assume the NS_VK_RETURN won't got generated so the JavaScript OnKeyUp
won't see it when we commit the Input Method.

Comment 12

[tague]

if the return key is being pressed to signal a commit, no virtual key event will
be generated for it.  if the return is pressed after a commit - a virtual key
will be generated

Comment 13

[Frank Tang]

tague, can you take this back since now is I doomed for IME :) I have trace down to the JavaScript in navigator.js
Look at BrowerLoadURL() . It first call document.getElementById('urlbar') and then got the value. I think you should set break point at

mozillay/layout/html/forms/src/nsFormControlFrame.cpp nsFormControlFrame::GetValue
and
http://lxr.mozilla.org/mozilla/source/xpfe/browser/src/nsBrowserInstance.cpp#954
954 nsBrowserAppCore::LoadUrl(const PRUnichar *aUrl)

to see which one damage the data

Comment 14

[tague]

bulk reassign bugs to ftang for now

Comment 15

[Frank Tang]

cata: can you help to look at this problem.

Comment 16

[Frank Tang]

bobj- please find someone to track this down...

Comment 17

[tao]

Turns out NS_NewURI(nsIURI* *result, const nsString& spec, nsIURI* baseURI)
is assuming all urlSpec are ascii encoded and calling nsString.ToNewCString()
 to get the internal buffer. The motivation might be to get a (char*) buffer
for the subsequent call to

  NS_NewURI(nsIURI* *result, const char* spec, nsIURI* baseURI)

The result is that non-ascii URL strings are truncated and displayed as garbage
in the URL bar area.

Comment 18

[tao]

Reassign to warren.

Comment 19

[tao]

IMO,

Either the NS_NewURI() interface needs to change to take PRUniChar* or the
PRUniChar* array need to be converted to UTF-8 before being passed as (char*).

Comment 20

[Warren Harris]

Jud,

Don't we do this conversion now? Can you take a look?

Warren

Comment 21

[Warren Harris]

NS_NewURI assumes that all urls are in UTF-8, not ASCII (at least that was the
plan). The webshell needs to translate before calling NS_NewURI.

Comment 22

[tao]

The file pointed by the URL field does not exist anymore. Replace it with a 
link to an article that addresses the need of fixing this bug.

Comment 23

[Judson Valeski]

over to ftang, he reworked a bunch of this recently.

Comment 24

[bobj]

warren> The webshell needs to translate before calling NS_NewURI.

Does this need to be kicked over to the webshell guys?

Comment 25

[Frank Tang]

The origional problem is gone. There are some other new issue now. Mark this bug 
fixed. 

Comment 26

[Frank Tang]

The origional problem is gone. mark this as fixed now. There are other issue 
still exist but outside the scope of this bug.

Comment 27

[Teruko Kobayashi]

I verified this in 2000041306 Win32 build.