URL bar assumes english keyboard input

VERIFIED FIXED in M16

Status

()

Core
Internationalization
P3
normal
VERIFIED FIXED
19 years ago
a year ago

People

(Reporter: tague, Assigned: Frank Tang)

Tracking

Trunk
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: NS_NewURI() assumes all urls are in ascii., URL)

(Reporter)

Description

19 years ago
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.

Updated

19 years ago
Component: Browser-General → Internationalization

Updated

19 years ago
Assignee: don → radha
Priority: P3 → P2
Target Milestone: M11

Comment 1

19 years ago
Radha, do we fix this or does the i18n team fix it?
I need some enlightment from the widget gurus and the I18N folks. cc'ing them.
Assignee: radha → ftang
Reassign to ftang. I think he knows better about the status of this.
(Assignee)

Updated

19 years ago
Assignee: ftang → tague
(Assignee)

Comment 4

19 years ago
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.

Updated

19 years ago
QA Contact: leger → teruko

Comment 5

19 years ago
Updating QA Contact.
(Assignee)

Updated

19 years ago
Assignee: tague → ftang
(Assignee)

Comment 6

19 years ago
tague is doomed, reassign this to myself.
(Assignee)

Updated

19 years ago
Target Milestone: M12 → M11
(Assignee)

Comment 7

19 years ago
Mark it as M11 assigned. Add akkana, buster to the cc list.
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 8

19 years ago
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.
(Assignee)

Comment 9

19 years ago
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.
(Assignee)

Comment 10

19 years ago
add radha/law and editors folks to the cc list.
(Assignee)

Comment 11

19 years ago
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.

Updated

19 years ago
Blocks: 15693
(Reporter)

Comment 12

19 years ago
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
(Assignee)

Updated

19 years ago
Priority: P2 → P3

Updated

19 years ago
Blocks: 16127
(Assignee)

Updated

19 years ago
Assignee: ftang → tague
Status: ASSIGNED → NEW
(Assignee)

Comment 13

19 years ago
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
(Reporter)

Updated

19 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 14

19 years ago
bulk reassign bugs to ftang for now
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED

Updated

19 years ago
No longer blocks: 16127
(Assignee)

Updated

19 years ago
Target Milestone: M11 → M12
(Assignee)

Updated

19 years ago
Assignee: ftang → cata
Status: ASSIGNED → NEW
(Assignee)

Comment 15

19 years ago
cata: can you help to look at this problem.
(Assignee)

Updated

19 years ago
Assignee: cata → bobj
(Assignee)

Comment 16

19 years ago
bobj- please find someone to track this down...

Updated

19 years ago
Status: NEW → ASSIGNED

Updated

19 years ago
Assignee: bobj → tao
Status: ASSIGNED → NEW
Target Milestone: M12 → M13

Comment 17

19 years ago
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.

Updated

19 years ago
Assignee: tao → warren
Whiteboard: NS_NewURI() assumes all urls are in ascii.

Comment 18

19 years ago
Reassign to warren.

Comment 19

19 years ago
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*).

Updated

19 years ago
Assignee: warren → valeski

Comment 20

19 years ago
Jud,

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

Warren

Updated

19 years ago
Target Milestone: M13 → M15

Comment 21

19 years ago
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

19 years ago
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

19 years ago
over to ftang, he reworked a bunch of this recently.
Assignee: valeski → ftang
Target Milestone: M15 → M16
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED

Comment 24

19 years ago
warren> The webshell needs to translate before calling NS_NewURI.

Does this need to be kicked over to the webshell guys?
(Assignee)

Comment 25

19 years ago
The origional problem is gone. There are some other new issue now. Mark this bug 
fixed. 
(Assignee)

Comment 26

19 years ago
The origional problem is gone. mark this as fixed now. There are other issue 
still exist but outside the scope of this bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 27

18 years ago
I verified this in 2000041306 Win32 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.