Closed Bug 128025 Opened 22 years ago Closed 20 years ago

show a confirm dialog when turning ON caret browsing (F7)

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.0

People

(Reporter: aaronlev, Assigned: aaronlev)

References

Details

(Keywords: access, topembed+, Whiteboard: [ADT2 RTM] [ETA Needed])

Attachments

(4 files, 2 obsolete files)

We don't want users getting into weird modes by pressing the wrong key.
When Ctrl+Shift+K is used to start browse with caret mode, a confirm dialog
should appear asking if they want to enter this mode. The default answer should
be No. There should be a checkbox, "Ask in the future"/
Blocks: 120023
Keywords: access, nsbeta1
nsbeta1+ per Chris Saari
Status: NEW → ASSIGNED
Keywords: nsbeta1nsbeta1+
Priority: -- → P2
Target Milestone: --- → mozilla1.0
Component: Browser-General → Keyboard Navigation
OS: Windows 2000 → All
QA Contact: doronr → sairuh
Hardware: PC → All
*** Bug 132860 has been marked as a duplicate of this bug. ***
I had to change the toggle key to F7 - something that would work on all locales
and platforms. Otherwise I wouldn't be able to inform the user in the confirm
dialog, what the toggle key to turn it back off again is. Since we're not
discoverable in any menu, this information is important.

Here is the message in the dialog:

Do you want to turn Caret Browsing on? This feature allows you to move around
the content with a cursor, and select text with the keyboard. Pressing F7
toggles it on and off.
[Use caret]	  [Cancel]
Correction, here is the exact wording of the dialog:

Do you want to turn Caret Browsing on? This feature allows you to move around
the content with a cursor, and select text with the keyboard. Pressing F7 turns
Caret Browsing on and off.

[ Use caret ]        [ Cancel ]
I have confirmed that this works in help, view source, inspect, mailnews, and
browser.
Attachment #76655 - Attachment is obsolete: true
r=akkana on everything except the browser.xml changes -- I don't understand
those well enough to review them.

I wonder how cheap/expensive it is to get the pref service every time we bring
up a new window.  In a lot of other places we make a static variable to cache
the prefs service, and it might be a good idea to do that here since it happens
on every window.
Akkana and I grepped through the source. It seems no one is caching the pref
service with a static in this way -- it doesn't seem to be an issue.
Comment on attachment 76762 [details] [diff] [review]
Forgot file - contains htmlBindings.xml which removes Accel+Shift+K binding

sr=hewitt
Attachment #76762 - Flags: superreview+
Comment on attachment 76762 [details] [diff] [review]
Forgot file - contains htmlBindings.xml which removes Accel+Shift+K binding

r=bryner
Attachment #76762 - Flags: review+
*** Bug 134144 has been marked as a duplicate of this bug. ***
cc'ing Jatin, to take a look at this dialog.
Comment on attachment 76762 [details] [diff] [review]
Forgot file - contains htmlBindings.xml which removes Accel+Shift+K binding

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #76762 - Flags: approval+
My suggested wording, if there is still time:

Pressing F7 turns Caret Browsing on or off. This feature places a moveable
cursor in web pages, allowing you to select text with the keyboard. Do you want
to turn Caret Browsing on?

[ Yes ]        [ Cancel ]
I have changed this to Jatin's wording. Question is whether I need r=/sr= or a=
again for the fixed wording.
Comment on attachment 77075 [details] [diff] [review]
Wording only change. Need new r= and a= only. Brendan gives blanked sr='s for wording only changes.

Adding brendan's universal sr= for wording changes
Attachment #77075 - Flags: superreview+
Comment on attachment 77075 [details] [diff] [review]
Wording only change. Need new r= and a= only. Brendan gives blanked sr='s for wording only changes.

r=bryner
Attachment #77075 - Flags: superreview+ → review+
Comment on attachment 77075 [details] [diff] [review]
Wording only change. Need new r= and a= only. Brendan gives blanked sr='s for wording only changes.

Adding sr= via brendan's universal sr= for wording-only changes
Attachment #77075 - Flags: superreview+
Keywords: adt1.0.0
Whiteboard: [ADT2]
a=marlon
adt1.0.0+, pending L10N and UE. Adding mcarlson, and lorikaplan to cc: list.
l10n approved. please check it in quick! thanks
Keywords: adt1.0.0adt1.0.0+
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
a=UI 
vrfy'd fixed using 2002.04.09 comm bits on linux rh7.2, win2k and mac 10.1.3.

clarifying the summary, since the dlg *only* appears when i turn *on* caret
browsing. (spoke to aaronl, who said that's by design.)
Status: RESOLVED → VERIFIED
Summary: When toggling browse with caret mode on, show a confirm dialog → show a confirm dialog when turning ON caret browsing (F7)
*** Bug 136905 has been marked as a duplicate of this bug. ***
This should be backed out, as it makes the feature completely inaccessible on
Mac OS.

(I could wonder how on earth this got UI approval when (a) it has buttons
labelled `Yes' and `Cancel' and (b) it includes a checkbox label ending in a
period, but such bugs are trivial in comparison.)
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Well, I collided with mpt... the only other comment I had was:

why is "BUTTON_TITLE_IS_STRING" used, then the string "Yes" is looked up?
_CANCEL is used for the other button... when not _YES?
This design doesn't mention what the keyboard shortcut is, because the string
would need to be made platform-specific (Shift+Command+K on the Mac,
Shift+Control+K elsewhere). Doing that isn't impossible (the Tabbed Browsing
panel in the prefs does it, for example), but not worth implementing, given
that this is a short-term fix until a proper fix is implemented where the alert
doesn't exist at all.

A proper fix would put this function in the prefs dialog (with no alert),
rather than having a keyboard shortcut, and would use Alt+Up/Down or similar
for normal scrolling when the function is turned on.
Aaronl, one more thing... why were the strings for the dialog placed in
tabbrowser.properties?

Do any other browsers include this accessability feature?
removing adt1.0.0+, as this bug has been reopened.
Blocks: 143047
Keywords: adt1.0.0+
Whiteboard: [ADT2] → [ADT2 RTM] [ETA Needed]
removing l10n approval since it's too late for UI changes in the branch.
tabbrowser.properties made the most sense for the string. I believe that
properties file is still included even in embedded projects, but we should check
to be sure. I didn't want to create a new properties file for this 1 string.

I agree this feature could use some tweaking. 

The reason a keystroke toggle is important, is that this feature can be used for
keyboard only users to go into a mode where they can select text, and then leave
that mode.

I don't mind mentionion "Start" as opposed to "Yes". However, I think it's
important to mention the keystroke so that people don't get lost in the feature.
Possibly the best idea is to make sure the feature always starts off when the
browser starts up. That way no one would ever get stuck in this.

This is an accessibility feature. I don't mind not having a great keystroke for
Mac users on an accessibility feature such as this, because Mac is still not an
accessible platform, far from it in fact. When Mac becomes accessible, I'll be
more inclined to worry about having quick toggles for accessibility features.
Therefore, it's a good time to use a function key.
mpt in comment 30:
> A proper fix would put this function in the prefs dialog (with no alert),
> rather than having a keyboard shortcut, and would use Alt+Up/Down or similar
> for normal scrolling when the function is turned on.

I'd really, really, really like this in the Edit or View menu as well.
Hello

Crashing:
When using F7, it is crashing the browser.
Pressing F7
toggles it on and off. It is crashing while turning it off 
In the mail program it is not doing this.

Mozilla 1.3a

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3a) Gecko/20021121

I see that this bug is set as blocking 1.3a so sorry if this is a dup report. 
<aol>Yeah, me too</aol>
Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.3a) Gecko/20030211 Phoenix/0.5

same with my colleague, he uses 1.3a on Windows XP.
Bug in Bugzilla.
While I do have my email address as CC in a few bugs,
I have never added myself to this bug's CC list.

I always post some comment while adding myself to a bug.
You will find no prier posting here from me.

I am removing myself from this bug's CC list and ,as always, posting
about it.

Have a nice day.
aaronl:
Warning: assignment to undeclared variable promptService
Source File: 
Line: 17

http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/browser.xml#348
Needs a var keyword.

Nothing significant on this bug has happened in a while; I can and will create
the patch for the warning momentarily.  But should this bug stay open?  Does it
crash current builds?  If so, can we get a Talkback ID?
Attachment #120347 - Flags: superreview?(bzbarsky)
Attachment #120347 - Flags: review?(timeless)
Comment on attachment 120347 [details] [diff] [review]
patch for strict warning

Next time, please do 'diff -u7' or more.  Context is good.  In this case it may
have let me not have to go look up the code in the file...
Attachment #120347 - Flags: superreview?(bzbarsky) → superreview+
ADT: Nominating topembed
Keywords: topembed
Discussed in top embed bug triage.  Plussing.  John AAron, can you do the review?
Keywords: topembedtopembed+
Comment on attachment 120347 [details] [diff] [review]
patch for strict warning

r=jgaunt
Attachment #120347 - Flags: review?(timeless) → review+
bug 207166 was submitted against Firebird to change Yes/Cancel to Yes/No.  The
following attachment is a patch for Firebird to address that request.  It also
replaces the BUTTON_TITLE_IS_STRING for the Yes to a simpler BUTTON_TITLE_YES
and removing the extra string from tabbrowser.properties.   Comment #28 here
noted the same issue.  Porting the patch is easy enough if you decide the
wording change is appropriate.

http://bugzilla.mozilla.org/attachment.cgi?id=126687&action=view
Mike, this will probably be fixed in bug 232512.

This bug here is open because of mpt's comment #27, not because of any crashes
(there are other bugs about that).

Alex, bz: that strict warning patch in attachment 120347 [details] [diff] [review] has reviews, but not
yet gone in. Can anybody check this in?
There is a confirm dialog.

If tweaks need to be made to the UI, please open a new clean bug.
Status: REOPENED → RESOLVED
Closed: 22 years ago20 years ago
Resolution: --- → FIXED
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: