GNOME Bugzilla – Bug 472345
Cannot arrow out of entries in FF3 if text is inserted via javascript
Last modified: 2007-10-12 15:36:17 UTC
Steps to reproduce: 1. Navigate to t-mobile.com 2. Tab until focus is on the entry after "My T-Mobile" (note that Orca will not say anything. That's another, independent bug.) 3. Press left arrow to try to escape the entry. Expected results: You could escape Actual results: You won't escape -- or if you do, you won't get very far. Seems we're getting caret-moved events for the entry that just lost focus as the rest of the text that is automatically added to that entry via javascript. See: https://bugzilla.mozilla.org/show_bug.cgi?id=394493
While we *could* add yet another "possibility #x" to onCaretMoved, this is a bogus event (the other possibilities are valid events). Bogus events are bad.
Created attachment 94773 [details] [review] if we have to hack.... On the bug we're tracking, Aaron said this: > I'm not sure if we'll get to this for Firefox 3 or not. > Can we consider it a "nice to have" and see where we > are with more important ones in 3-4 weeks? > > Or do you see this on a lot of pages. So I told him we see it with a lot of pages. This really is cropping up more and more. Stupid shiny javascript. :-( Anyhoo, here's the "just in case" <strike>hack</strike> patch for whatever it's worth.
This looks like a safe hack with high user benefit. If you and Mike have tested this, I say commit for trunk and the gnome-2-20 branch.
Patch committed to both. Moving to pending.
I just noticed that I can left arrow out of this field but not right arrow.
Seems to be a different bug. Investigating now.
Created attachment 95153 [details] [review] fix right arrowing out of entries; deal with more javascript goofiness You can't right arrow out of any empty entries it seems. And the reason why seems to be that empty entries aren't really empty. They have a length of 1 whose text is the newline char. My guess is that the code in useCaretNavigationModel() that says weHandleIt if we're in a 0-length entry was meant to handle empty entries. Therefore, also check for the "it looks empty but it's really not" case. So then I was happily testing our apparent newfound ability to arrow into and out of these beasts in both directions and discovered a traceback when left arrowing from the password entry to the phone # entry. I'm still working out all the goofiness, but it seems to boil down to this: Text is getting magically inserted and deleted as we innocently navigate among these entries and we're not updating our caretContext -- either at all or fast enough (not sure which yet) -- and asking for characters using offsets that were valid a second ago, but no longer are. In the long run we need to work out exactly what is going on and identify the best way to deal with it. In the meantime, I added on a couple of sanity checks. We can at least arrow into and out of these blessed javascripty miracles without errors or getting stuck. I think. Mike please test.
Hmmm.... We're saying "newline" now when you arrow into entries that are actually empty. Hold on. Let me try again.
Okay, it turns out that if you right arrow into an entry even without my patch we say "newline" so that's definitely a different bug. So back to the original, Mike please test.
Seems to work now. thanks
Thanks Mike. I'm leaning towards this being a post 2.20 thing given how close we are to code freeze. I'd hate for there to be some unanticipated side effect that we didn't catch in time. We can also open new bugs to deal with the fact that we say "newline" when we right arrow into an empty entry and to address the aforementioned goofy text insertions and deletions. Thoughts?
If you leave this fix out of 2.20 are you still going to leave the fix in place for moving left out of these controls?
That was my thinking. Although, then we'll potentially get the tracebacks going from the password to the phone #.... <frown> Will what should we do?
(In reply to comment #13) > That was my thinking. Although, then we'll potentially get the tracebacks > going from the password to the phone #.... <frown> Will what should we do? > We need to put our pencils down for 2.20.0. With the gnome-2-20 branch as is, Orca doesn't die, the user can recover, and this only occurs on some ill-behaved websites. We can look at this post 2.20.0, possibly for 2.20.1.
Changing the status of this patch to needs-work because we need to look at what to do about our caret context being removed out from under us. :-( See comment #7.
I'm taking another look at this now. 1. The defensive work is still needed. 2. Having the proverbial rug pulled out from under us is a broader bug (a la the 411.com bug, for which I now need to create a new test case). This bug is merely about not being able to arrow out of entries. Therefore, I'm putting this back on the table for consideration. :-) Will please review.
This patch seems simple enough. If you've tested it like crazy and it seems to do the trick, I say submit it to trunk and gnome-2-20 with one minor consideration. I'm not sure that "=" should be in there: + if characterOffset <= len(unicodeText): + return unicodeText[characterOffset].encode("UTF-8") >>> a = "oops" >>> a[len(a)] Traceback (most recent call last):
+ Trace 169508
Created attachment 97061 [details] [review] Fixed stupid mistake. (Thanks Will!) >>> joanie = "idiot" >>> joanie 'idiot' Sorry 'bout that!
Thanks!
Created attachment 97063 [details] [review] found a harmless redundancy + if (startOffset == -1) or (startOffset >= len(unicodeText)): startOffset = len(unicodeText) Should be: + if (startOffset == -1) or (startOffset > len(unicodeText)): startOffset = len(unicodeText) I really need more coffee and more sleep....
Committed to both trunk and the gnome-2-20 branch. Moving to pending.
I think this one looks good now.
Thanks Mike! Closing as FIXED.