Closed Bug 1053462 Opened 5 years ago Closed 5 years ago

Add fromUserInput field to pivot move events and move methods

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: eeejay, Assigned: eeejay)

Details

Attachments

(1 file)

Basically, we should add a boolean argument to move methods to allow setting fromUserInput flag to false (default is true).

If 'position' field is set directly, isFromUserInput should be false.

I'm going to have all text methods be true for now, and perhaps add them later when we use those methods more.
Comment on attachment 8472623 [details] [diff] [review]
Add isFromUserInput to vc change events and pivot methods.

Review of attachment 8472623 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/base/AccEvent.h
@@ +469,5 @@
>    AccVCChangeEvent(Accessible* aAccessible,
>                     nsIAccessible* aOldAccessible,
>                     int32_t aOldStart, int32_t aOldEnd,
> +                   int16_t aReason,
> +                   EIsFromUserInput aIsFromUserInput = eFromUserInput);

no autodetection for VCChangeEvent?

::: accessible/base/nsAccessiblePivot.cpp
@@ +100,5 @@
>    mPosition.swap(secondPosition);
>    int32_t oldStart = mStartOffset, oldEnd = mEndOffset;
>    mStartOffset = mEndOffset = -1;
>    NotifyOfPivotChange(secondPosition, oldStart, oldEnd,
> +                      nsIAccessiblePivot::REASON_NONE, false);

as we discussed it'd be good to move the arg into interface

::: accessible/interfaces/nsIAccessiblePivot.idl
@@ +194,5 @@
> +   * @param aOldStart        [in] the old start offset, or -1.
> +   * @param aOldEnd          [in] the old end offset, or -1.
> +   * @param aReason          [in] the reason for the pivot change.
> +   * @param aIsFromUserInput [in] called because of direct user input
> +   *                           (default is true).

maybe like: "the pivot change because of ...", just "called" may sound a bit confusing

::: accessible/interfaces/nsIAccessibleVirtualCursorChangeEvent.idl
@@ +8,5 @@
>  /*
>   * An interface for virtual cursor changed events.
>   * Passes previous cursor position and text offsets.
>   */
> +[scriptable, builtinclass, uuid(96d2355b-c382-4bfa-bebc-3ef6a130dd03)]

what's for?

::: accessible/tests/mochitest/pivot.js
@@ +205,5 @@
>   * @param aPivotMoveMethod [in] method to test (ie. "moveNext", "moveFirst", etc.)
>   * @param aRule            [in] traversal rule object
>   * @param aIdOrNameOrAcc   [in] id, accessible or accessible name to expect
>   *                         virtual cursor to land on after performing move method.
>   *                         false if no move is expected.

missed new arg descripiton

@@ +228,5 @@
> +        case 'movePrevious':
> +          moved = aDocAcc.virtualCursor[aPivotMoveMethod](aRule,
> +            aDocAcc.virtualCursor.position, false,
> +            aIsFromUserInput === undefined ? true : false);
> +          info('moving!! ' + moved)

do you need 'info'?
Attachment #8472623 - Flags: review?(surkov.alexander) → review+
(In reply to alexander :surkov from comment #2)
> Comment on attachment 8472623 [details] [diff] [review]
> Add isFromUserInput to vc change events and pivot methods.
> 
> Review of attachment 8472623 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/base/AccEvent.h
> @@ +469,5 @@
> >    AccVCChangeEvent(Accessible* aAccessible,
> >                     nsIAccessible* aOldAccessible,
> >                     int32_t aOldStart, int32_t aOldEnd,
> > +                   int16_t aReason,
> > +                   EIsFromUserInput aIsFromUserInput = eFromUserInput);
> 
> no autodetection for VCChangeEvent?

No, we are by default from user input. EventStateManager::IsHandlingUserInput() doesn't give us anything useful.
https://hg.mozilla.org/mozilla-central/rev/abb6a2af7cb5
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.