The current mozilla::ScrollAxis struct in PresShellForwards.h isn't a simple
axis enum. It carries information about where to scroll to along a specific
axis, and under what conditions that should happen. So 'ScrollAxis' isn't the
best name.
Now we're introducing WebIDL 'ScrollAxis' in bug 1350457:
https://drafts.csswg.org/scroll-animations-1/#scrolltimeline-interface
The extensive use of PresShellForwards.h's ScrollAxis in DOM code makes it
conflict with this new DOM enum. Rather than try to fix this through lots of
explitic namespacing, this patch renames PresShellForwards.h's ScrollAxis.
Differential Revision: https://phabricator.services.mozilla.com/D293485
As tested by `css/css-ui/user-select-none-in-editable.html`, we should
treat editable nodes as selectable even if `user-select: none` is
specified.
Then, some tests start failing due to an existing bug of caret move
when an inline element starts/ends with invisible white-spaces.
Therefore, to make them keep passing, this fixes the bug of
`nsTextFrame::PeekOffsetCharacter()`.
Differential Revision: https://phabricator.services.mozilla.com/D272608
Use inline friend operator declaration, which in turns disable
comparison between iterators over integer of different types.
Also remove useless inclusion in the process.
Differential Revision: https://phabricator.services.mozilla.com/D267876
Previously, we did this for insertion and removal, but not relocation.
This meant that if something queried HyperText offsets soon after a child was relocated, the HyperText offset cache could be stale.
This could result in very broken expectations such as an embedded object having more than 1 offset.
This was causing an assertion, but could likely result in other obscure problems too.
Differential Revision: https://phabricator.services.mozilla.com/D259210
Moved GetCaretRect to HyperTextAccessibleBase. I could see this living
in TextLeafPoint, but it is a bit confusing when a point is a caret and
when it is a simple offset. I think we would need to either add an
mIsCaret field to the class, or have a special static CaretBounds
method.
Differential Revision: https://phabricator.services.mozilla.com/D256377
Moved GetCaretRect to HyperTextAccessibleBase. I could see this living
in TextLeafPoint, but it is a bit confusing when a point is a caret and
when it is a simple offset. I think we would need to either add an
mIsCaret field to the class, or have a special static CaretBounds
method.
Differential Revision: https://phabricator.services.mozilla.com/D256377
These are implemented in HyperTextAccessibleBase.
Having local and remote use the same implementation makes things simpler, more consistent, easier to test and easier to reason about.
This is also necessary in order to properly test the fix in the next patch.
This breaks the rare edge case of selecting only empty containers for LocalAccessible.
This never worked for RemoteAccessible anyway, which is the majority of content now.
Bug 1973154 has been filed for this.
These functions were the only callers of HyperTextAccessible::GetSelectionDOMRanges, so that has been removed as well.
However, GetSelectionDOMRanges had an early return to deal with bug 1840574.
That early return has been migrated to HyperTextAccessible::SelectionRanges, which is called by the HyperTextAccessibleBase implementations of SelectionCount and SelectionBoundsAt, thus ensuring that we still handle the case in bug 1840574.
Differential Revision: https://phabricator.services.mozilla.com/D254466
I removed this behaviour in bug 1951573.
Unfortunately, it turns out that contrary to the spec (which says "1 or more selections"), NVDA depends on this behaviour.
Reinstate this by supporting kRemoveAllExistingSelectedRanges in HyperTextAccessibleBase::RemoveFromSelection and using that.
Differential Revision: https://phabricator.services.mozilla.com/D247875
This was causing an assertion when firing caret events where the element containing the caret is aria-hidden, since the offset we produced was effectively bogus for the target Accessible.
Doing this is bad authoring, but we need to at least ensure the offset is valid.
Arguably, we shouldn't fire caret events at all in this case, but this is a trickier, riskier fix and I don't think it makes sense to spend time on that to compensate for bad authoring.
Differential Revision: https://phabricator.services.mozilla.com/D240579
There were still some callers of SetSelectionRange, but these can now all be trivially converted to use SetSelectionBoundsAt (which calls TextLeafRange::SetSelection).
Differential Revision: https://phabricator.services.mozilla.com/D240277
Sometimes, text in the accessibility tree can be out of date with DOM briefly because the accessibility engine updates text asynchronously.
When this happens, DOMPointToOffset can be called on a text node with an offset which is valid for DOM, but not yet valid for accessibility.
Previously, we ended up producing an invalid HyperText offset, causing a crash.
The best we can do in this case is gracefully constrain the offset to the text in the accessibility tree.
In the scenario in this bug report, the user was typing into the address bar.
The address bar added and selected text due to an autocomplete suggestion.
After the DOM update but before the accessibility tree was updated, a UIA client queried the selection.
Since selection is retrieved from DOM, this resulted in an offset that didn't exist in the accessibility tree.
Now that the offset is constrained, this will no longer result in a crash.
Differential Revision: https://phabricator.services.mozilla.com/D238413
Usually, we name methods as `GetFoo()` if they may return `nullptr`, on the
other hand, we name methods as `Foo()` if they won't return `nullptr`.
`RangeBoundaryBase::Container()` may return `nullptr`. So, this name may cause
misleading of some developers. Let's rename it as `GetContainer()`.
This does not change anything for avoiding merge conflict with the other
landings.
Differential Revision: https://phabricator.services.mozilla.com/D236796
There are some callers of parent/offset version which computes offset before.
However, the offset may not be used. Therefore, the callers should use
`RangeBoundaryBase` version.
Additionally, if only one of the pairs is computed from `RangeBoundaryBase`,
such callers should use the `RangeBoundaryBase` version too because the offset
computation may be skipped. In this case, temporary `RangeBoundaryBase` should
be a `RawRangeBoundary` and whose `aRangeIsMutationObserver` should be set to
`RangeBoundaryIsMutationObserved::No` to avoid immediately to compute the child
node from `aOffset`. I think that this should be default to
`RangeBoundaryIsMutationObserved::No` in the future, but for now, we should just
make the users explicitly set it to `RangeBoundaryIsMutationObserved::No` for
avoiding regressions.
Differential Revision: https://phabricator.services.mozilla.com/D236793
`nsFrameSelection::NormalSelection()` returns a `Selection&`, which eliminates the need for nullptr check at call sites.
This simplifies the code in some places and removes unnecessarily indented blocks.
Differential Revision: https://phabricator.services.mozilla.com/D230583
If no text is selected, this is the same as passing the caret offset, since the selection range is already where we want it.
However, if text is selected, this allows the caller to replace it, just as it would be when pasting using the keyboard or GUI.
Note that this doesn't break any existing expectations because TEXT_OFFSET_CARET was never supported for this method previously.
This allows us to remove the DeleteText call from Android SessionAccessibility::Paste.
This separate DeleteText call was causing problems because it could result in the deletion and the insertion happening in two separate ticks, resulting in incorrect events.
Now, this should always happen in a single tick.
Differential Revision: https://phabricator.services.mozilla.com/D226108
These loops may run script.
In `HyperTextAccessible`, it calls
`RemoveRangeAndUnselectFramesAndNotifyListeners`. So, chrome script which is not
directly related to this module may run in each call. I think that using
`RemoveAllRanges` is better for here. Before bug 1735446, this loop tried
to keep first range, but accidentally, I changed this loop delete all ranges.
However, `SetSelectionBoundsAt` will add a range if it's required and there is
no bug reports. Therefore, I think keep the new behavior is better.
In `nsRange::DeleteFromDocument`, the loop may run mutation events too.
Therefore, it needs to store all ranges first. Then, the preceding patch
changes the behavior here if a selection range is moved to different root.
Previously, it was deleted, but now, they are not touched.
Depends on D200606
Differential Revision: https://phabricator.services.mozilla.com/D200607
The methods are in `nsCaret`, `nsFrameSelection` and `dom::Selection`. That
makes harder to read, and they are called each other, so, they are reused for
different purpose. Therefore, it must be better to move them into a new class.
Then, the name differences may become more unclear. If you think so, feel free
to request renaming some methods of them.
Differential Revision: https://phabricator.services.mozilla.com/D197288
Role.h will soon be generated, but it is generated within the obj dir, so local includes won't work.
Our C++ style guide says we should prefer exported includes wherever possible anyway.
This was done with this shell command inside the accessible/ directory:
```
sed -i 's,#include "Role.h",#include "mozilla/a11y/Role.h",' `git grep -l '#include "Role.h"'`
```
Differential Revision: https://phabricator.services.mozilla.com/D183940
This revision removes unnecessary include directives from cpp files in the
accessible/generic directory. These suggestions came from the Include What You
Use tool.
Depends on D182397
Differential Revision: https://phabricator.services.mozilla.com/D182398
This patch does several things:
1. Remove virtual cursor doc accessible member.
2. Remove state-keeping nsAccessiblePivot implementation and replace it
with something that is much more similar to the native Pivot
interface.
3. Move nsIAccessiblePivot implementation to xpcom and rename it
xpcAccessiblePivot.
4. Delete mochitests and introduce browser tests.
5. Make new xpcom implementation work on remote accessibles as well.
Differential Revision: https://phabricator.services.mozilla.com/D181813
This patch does several things:
1. Remove virtual cursor doc accessible member.
2. Remove state-keeping nsAccessiblePivot implementation and replace it
with something that is much more similar to the native Pivot
interface.
3. Move nsIAccessiblePivot implementation to xpcom and rename it
xpcAccessiblePivot.
4. Delete mochitests and introduce browser tests.
5. Make new xpcom implementation work on remote accessibles as well.
Differential Revision: https://phabricator.services.mozilla.com/D181813
This patch does several things:
1. Remove virtual cursor doc accessible member.
2. Remove state-keeping nsAccessiblePivot implementation and replace it
with something that is much more similar to the native Pivot
interface.
3. Move nsIAccessiblePivot implementation to xpcom and rename it
xpcAccessiblePivot.
4. Delete mochitests and introduce browser tests.
5. Make new xpcom implementation work on remote accessibles as well.
Differential Revision: https://phabricator.services.mozilla.com/D181813
This patch does several things:
1. Remove virtual cursor doc accessible member.
2. Remove state-keeping nsAccessiblePivot implementation and replace it
with something that is much more similar to the native Pivot
interface.
3. Move nsIAccessiblePivot implementation to xpcom and rename it
xpcAccessiblePivot.
4. Delete mochitests and introduce browser tests.
5. Make new xpcom implementation work on remote accessibles as well.
Differential Revision: https://phabricator.services.mozilla.com/D181813
We have more readable and faster versions (that just omit the namespace
arg).
Mostly done via sed, with a couple helpers to use the faster lookups
where possible.
Differential Revision: https://phabricator.services.mozilla.com/D181795
Leo Tenenbaum