Last Comment Bug 672316 - accessibility.browsewithcaret is sometimes enabled when it shouldn't be
: accessibility.browsewithcaret is sometimes enabled when it shouldn't be
Status: VERIFIED FIXED
: regression, verified-aurora
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 7
Assigned To: Matt Brubeck (:mbrubeck)
:
Mentors:
Depends on: 667243
Blocks: 661388
  Show dependency treegraph
 
Reported: 2011-07-18 12:30 PDT by Matt Brubeck (:mbrubeck)
Modified: 2011-07-25 08:20 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.85 KB, patch)
2011-07-18 12:30 PDT, Matt Brubeck (:mbrubeck)
mark.finkle: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Review

Description Matt Brubeck (:mbrubeck) 2011-07-18 12:30:24 PDT
Created attachment 546601 [details] [diff] [review]
patch

I don't have reliable steps to reproduce this, but I find that caret browsing is sometimes enabled unexpectedly after I've been using Fennec for a while.  Looking at the code, this could happen if an error occurs in SelectionHelper.hide before the pref is unset.  This patch just adds a try block to hopefully catch these errors.
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-07-18 13:18:41 PDT
Comment on attachment 546601 [details] [diff] [review]
patch

I guess the popupState could be null or something
Comment 2 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-07-18 13:55:53 PDT
Isn't this basically like bug 671052 (which was duplicated against bug 667243)?

Also, isn't it bad to add try..catch to work around a problem?
Comment 3 Matt Brubeck (:mbrubeck) 2011-07-18 14:29:27 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/00df362906e9

(In reply to comment #2)
> Isn't this basically like bug 671052 (which was duplicated against bug
> 667243)?

Similar, though in my case the caret appeared after the handles were gone.

> Also, isn't it bad to add try..catch to work around a problem?

Yes, this is a slight hack because I could not reproduce the bug to be sure exactly where the error occurred.  This code will be removed when the real fix (bug 667243) is available.
Comment 4 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-07-18 14:32:41 PDT
Ok, adding dependancy to bug 667243 then.
Comment 5 Marco Bonardo [::mak] 2011-07-19 08:09:24 PDT
http://hg.mozilla.org/mozilla-central/rev/00df362906e9
Comment 6 Matt Brubeck (:mbrubeck) 2011-07-19 08:13:24 PDT
Comment on attachment 546601 [details] [diff] [review]
patch

Requesting approval-mozilla-aurora.  This change is mobile-only and fixes a regression in Firefox 7.  The change is very low-risk; it just wraps a try/catch around an existing line of code.
Comment 7 Matt Brubeck (:mbrubeck) 2011-07-20 07:14:01 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/e0ad9f840fa6
Comment 8 Aaron Train [:aaronmt] 2011-07-25 08:20:52 PDT
Verified Fixed

Nightly: Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110725 Firefox/8.0a1 Fennec/8.0a1
Aurora: Mozilla/5.0 (Android; Linux armv7l; rv:7.0a2) Gecko/20110725 Firefox/7.0a2 Fennec/7.0a2

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