Closed Bug 476897 Opened 11 years ago Closed 11 years ago

need ability to be disable F7 caret browsing shortcut

Categories

(Toolkit :: XUL Widgets, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: taras.mozilla, Assigned: taras.mozilla)

References

Details

Attachments

(2 files, 4 obsolete files)

No description provided.
Attached patch new pref (obsolete) — Splinter Review
Adds accessibility.disable_browsewithcaret pref
Attachment #360547 - Flags: review?(gavin.sharp)
I think the pref would be best named "accessibility.browserwithcaret_shortcut.enabled", with a default value of true in all.js, no try/catches, and no temporary "disabled" attribute.
Assignee: nobody → tglek
Attached patch rev2 (obsolete) — Splinter Review
Attachment #360547 - Attachment is obsolete: true
Attachment #360575 - Flags: review?(gavin.sharp)
Attachment #360547 - Flags: review?(gavin.sharp)
Comment on attachment 360575 [details] [diff] [review]
rev2

You don't like semi-colons, eh? :) Please add one on both lines.

I'd also skip the temporary and just put the getBoolPref call in the if(), but r=me either way (but do add a semi-colon after |return|).
Attachment #360575 - Flags: review?(gavin.sharp) → review+
Attached patch added ; (obsolete) — Splinter Review
Can you stick an r+ on the updated patch
Attachment #360575 - Attachment is obsolete: true
Attachment #360757 - Flags: review?(gavin.sharp)
Keywords: checkin-needed
Comment on attachment 360757 [details] [diff] [review]
added ;

>diff --git a/toolkit/content/widgets/browser.xml b/toolkit/content/widgets/browser.xml

>+          var isEnabled = this.mPrefs.getBoolPref("accessibility.browserwithcaret_shortcut.enabled")

Still missing a ; here :)
Attachment #360757 - Flags: review?(gavin.sharp) → review+
Attached patch added the other ; (obsolete) — Splinter Review
lets try this again. I'm sorry that my genetic predisposition against ; is affecting me this much.
Attachment #360757 - Attachment is obsolete: true
Attachment #360758 - Flags: review?(gavin.sharp)
Attachment #360758 - Flags: review?(gavin.sharp) → review+
Phil points out that the pref name should contain "browsewithcaret", and not "browserwithcaret". Need to fix that before pushing!
Landed as 2accea7cd21d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: wanted1.9.1?
Shouldn't the pref-name also be renamed?
(In reply to comment #11)
> Shouldn't the pref-name also be renamed?

Yes

+pref("accessibility.browserwithcaret_shortcut.enabled", true);

should be:

+pref("accessibility.browsewithcaret_shortcut.enabled", true);
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch fixupSplinter Review
Sorry about that
https://hg.mozilla.org/mozilla-central/rev/3917a3ac2b78
Keywords: checkin-needed
OS: Linux → All
Hardware: x86 → All
Summary: Need to be able to disable F7 caret browsing shortcut for Fennec → need ability to be disable F7 caret browsing shortcut
Target Milestone: --- → mozilla1.9.2a1
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Duplicate of this bug: 334169
You need to log in before you can comment on or make changes to this bug.