Last Comment Bug 135196 - F7 to toggle browse-with-caret hits a JS error
: F7 to toggle browse-with-caret hits a JS error
Status: VERIFIED FIXED
[adt2]
: embed
Product: Core
Classification: Components
Component: Keyboard: Navigation (show other bugs)
: Trunk
: x86 Windows 98
: P1 critical (vote)
: mozilla1.0
Assigned To: Aaron Leventhal
: sairuh (rarely reading bugmail)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2002-04-03 10:52 PST by Jesse Ruderman
Modified: 2002-04-08 21:58 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Wraps F7 handler in try/catch. (3.52 KB, patch)
2002-04-03 13:22 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Better patch -uses multiple try/catch's (2.94 KB, patch)
2002-04-03 14:43 PST, Aaron Leventhal
bzbarsky: review+
hewitt: superreview+
asa: approval+
Details | Diff | Splinter Review

Description Jesse Ruderman 2002-04-03 10:52:45 PST
When I press F7 in 2002 040303, nothing happens in the browser window, and the
following appears on the JS console:

Error: uncaught exception: [Exception... "Component returned failure code:
0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]"  nsresult:
"0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: <unknown filename> ::
onxblkeypress :: line 4"  data: no]
Comment 1 Aaron Leventhal 2002-04-03 11:12:55 PST
Strange. Can you look in your all.js file and see if it has the lines:
pref("accessibility.browsewithcaret", false);
pref("accessibility.warn_on_browsewithcaret", true);

Is this a downloaded binary build or compiled from source?
Comment 2 Boris Zbarsky [:bz] 2002-04-03 11:56:43 PST
umm... no matter what the default all.js is, distributors can configure it. 
Code should always wrap get*Pref calls in try/catch
Comment 3 Aaron Leventhal 2002-04-03 12:01:19 PST
Ok, now I know. Accepting.
Comment 4 Aaron Leventhal 2002-04-03 12:59:19 PST
Interesting, I think we don't use try/catch in a lot of places where we get
prefs, for example tabbrowser.xml.
Comment 5 Aaron Leventhal 2002-04-03 13:22:54 PST
Created attachment 77518 [details] [diff] [review]
Wraps F7 handler in try/catch.

Still not sure why Jesse was getting exceptions. Curious whether his all.js
file has the lines I mentioned.
Comment 6 Boris Zbarsky [:bz] 2002-04-03 13:35:49 PST
doesn't that just silently ignore F7 and not go into browse-with-caret mode
(since all the code is in thee try)?

I'd think that a better approach would be:

var foo = true;
try {
  foo = mPrefs.getBoolPref("accessibility.warn_on_browsewithcaret");
} catch (ex) {}
if (foo && !browseWithCaret) {
  // go on 
}

And similarly for other pref accesses.
Comment 7 Aaron Leventhal 2002-04-03 14:43:11 PST
Created attachment 77535 [details] [diff] [review]
Better patch -uses multiple try/catch's
Comment 8 Jesse Ruderman 2002-04-03 14:48:22 PST
The 2002 040303 I tested is a nightly talkbalk zip build for Windows.  I 
installed it in a new directory, like I usually do when I get a new build.

I checked all.js and only saw
pref("accessibility.browsewithcaret", false);
Comment 9 Boris Zbarsky [:bz] 2002-04-03 14:53:46 PST
Comment on attachment 77535 [details] [diff] [review]
Better patch -uses multiple try/catch's

Wouldn't it be better to have browseWithCaretOn default to "false"?  Do that
and r=bzbarsky
Comment 10 Aaron Leventhal 2002-04-03 14:58:17 PST
Done.

The reason the exception happened in the first place, is that I forgot to check
in my all.js changes! Duh.

Anyway, the try/catch is an improvement as well.
Comment 11 Joe Hewitt (gone) 2002-04-03 15:10:15 PST
Comment on attachment 77535 [details] [diff] [review]
Better patch -uses multiple try/catch's

yeah, default that to false

sr=hewitt
Comment 12 Jaime Rodriguez, Jr. 2002-04-03 17:21:43 PST
adt1.0.0+ (on ADT's behalf) for approval for checkin.
Comment 13 Asa Dotzler [:asa] 2002-04-03 20:01:45 PST
Comment on attachment 77535 [details] [diff] [review]
Better patch -uses multiple try/catch's

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Comment 14 Aaron Leventhal 2002-04-04 15:27:03 PST
checked in
Comment 15 sairuh (rarely reading bugmail) 2002-04-08 21:58:01 PDT
i don't see a js error when using F7. vrfy'd using 2002040510 linux comm bits.

Note You need to log in before you can comment on or make changes to this bug.