Previously, DoInitialUpdate built the initial tree and sent it over IPC.
aria-owns relocations were processed later in WillRefresh.
This caused two problems.
First, it meant that we sent the initial tree over IPC and then immediately sent a bunch of mutations for the aria-owns relocations, as well as firing client events for those mutations, which is wasteful.
Second, because tagged PDF generation only performs the initial update and expects the document to be completely ready thereafter, this meant that our tagged PDF output didn't take aria-owns into account.
To fix this, process the initial aria-owns relocations as part of DoInitialUpdate, before we send the tree over IPC.
We don't want to queue mutation events as part of this, as this would be redundant and break IPC expectations.
To avoid that, we set eTreeConstructed after processing relocations and avoid queuing events in various places if that flag isn't set.
Differential Revision: https://phabricator.services.mozilla.com/D286838
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
Previously, if there was an anchor jump to a DOM node that wasn't yet rendered in the accessibility tree, we could fail in one of two ways:
1. If none of the target's ancestors (other than the document itself) were in the accessibility tree, we wouldn't fire a scrolling start event until the document was next re-focused.
2. If the target did have a container already in the accessibility tree, we would fire a scrolling start event on the container, which might effectively be a long way from the target.
To fix this, if there is a pending tree update, process the anchor jump only after the tree is up to date (in WillRefresh).
As part of this, anchor jump processing has been refactored into a single, unified function.
Aside from avoiding additional code duplication, this ensures that anchor jump handling is consistent going forward, as there are quite a few subtle details to get right which have bitten us in the past; e..g bug 1926198.
Differential Revision: https://phabricator.services.mozilla.com/D255274
Run it in the right step of the rendering loop as per spec. Keep
the interruptible layout behavior by detecting whether we have any
frame with content-visibility: auto / ResizeObserver / etc around.
Also make the IntersectionObserver phase not unconditional, make it
conditional on needing layout or having scrolled.
I want to look a bit whether we can avoid flushing the whole browsing
context tree, but that is pre-existing so not touching it as part of
this work.
Differential Revision: https://phabricator.services.mozilla.com/D245214
In bug 1951067, I pruned all 0 width whitespace text nodes from the accessibility tree.
However, it turns out that whitespace text nodes at the end of wrapped lines can also be 0 width.
These have semantic importance, since otherwise, words in separate inline nodes can be merged together without a space.
To fix this, explicitly check that the node is before a hard line break, not a soft (wrapped) line break.
Differential Revision: https://phabricator.services.mozilla.com/D242290
I placed them in a way that will ensure the correct order of events and
not allow menu start or alert events to get fired before the show, and
not allow menu end events to fire after the hide.
Differential Revision: https://phabricator.services.mozilla.com/D235188
This revision modifies the way that ProcessMutationEvents works. Rather than
firing mutation events immediately, it instead causes them to be queued on
DocAccessibleChild via HandleAccEvent. The code then fires the events in batches
with a new IPC method, SendMutationEvents. The net effect of this change is a
significant reduction in pressure on the IPC system, in favor of fewer, larger
messages. For pages that generate many mutation events often, this represents a
big win for performance. Without this, the IPC queue can get backed up, failing
to keep up with the amount of accessibility IPC messages as they flood in.
The method of batching is carefully written to maximize the amount of
accessibles that can be touched in a mutation event batch without going
over the IPC message size limit. This revision does not implement size
tracking, but does make a reasonable approximation. With these changes,
heterogenous mutation event types can sit together in batches, including
sub-batches of show events. A side effect of this change is that initial
page load is further amalgamated; multiple document children that
produce separate calls to InsertIntoIpcTree now use this queueing
mechanism and can have their show events sent together.
Some messages may fire differently from their former ordering with this change,
particularly EVENT_MENUPOPUP_END for content processes. However, that event
appears to have been fired out of order with the existing code, meaning
that this change represents a fix to that ordering issue as well.
Finally, this revision makes a slight modification to an Android test
for clarity and correctness. I discovered this issue while debugging
event ordering and realized it was valuable to fix.
Differential Revision: https://phabricator.services.mozilla.com/D229842
Since the method is deferred we need to do extra guesswork for possible
situtations where the name has changed because we don't have the
privilege to calculate the name in-line when content is deleted.
I tried to account for all cases as we have in our test coverage. I
hope that if there are edge cases they are false positives, and we are
firing extra name changes and not the opposite.
Differential Revision: https://phabricator.services.mozilla.com/D223877
Since the method is deferred we need to do extra guesswork for possible
situtations where the name has changed because we don't have the
privilege to calculate the name in-line when content is deleted.
I tried to account for all cases as we have in our test coverage. I
hope that if there are edge cases they are false positives, and we are
firing extra name changes and not the opposite.
Differential Revision: https://phabricator.services.mozilla.com/D223877
When we create a TextLeafAccessible for a CSS pseudo-element, we already use the alt text if present.
However, it seems layout sometimes re-renders the text of pseudo-elements.
Previously, a11y would update the text in this case like it would for any other TextLeafAccessible, thus overwriting the alt text with the text rendered by layout.
To fix this, we now skip updating text if CSS alt text is present.
Differential Revision: https://phabricator.services.mozilla.com/D211139
There's no call to AddRefreshObserver(FlushType::Layout), so we don't
need to track layout flushes. The only reason we need
mLayoutFlushObservers is so that reflows triggered from the style flush
get processed. But at that point, we can just flush layout directly.
There's no really good distinction between style flushes and layout
flushes with container queries anyways, so this makes the code simpler
to reason about.
Differential Revision: https://phabricator.services.mozilla.com/D209924
ContentRemoved recursively walks both AllChildrenIterator and direct DOM children.
In addition, we might get duplicate notifications from DOM and layout, plus PruneOrInsertSubtree might do a recursive walk and it too calls ContentRemoved.
To avoid this duplicate processing, keep a set of removed DOM nodes on the DocAccessible which we clear after mutation events are processed.
Differential Revision: https://phabricator.services.mozilla.com/D196707
std::sort used by nsTArray::Sort expects the comparator to be tolerant
for being called to compare the very same element with itself.
Differential Revision: https://phabricator.services.mozilla.com/D195482
This is just a "belt-and-suspenders" invariant tightening. Shouldn't impact
behavior at all, assuming that we don't call methods on NotificationController
objects that have been nerfed with Shutdown().
Differential Revision: https://phabricator.services.mozilla.com/D195321
If these assertions somehow fail, then we are probably doomed to crash anyway.
The new release assertions just make it so we'll crash in a controlled way,
with a more actionable backtrace, closer to the problem-spot.
This patch also removes one mDocument null-check that's becoming redundant
since it follows a release-assert that would make us abort if the pointer is
null.
Differential Revision: https://phabricator.services.mozilla.com/D195042
We can sometimes reach the code that this patch is touching, just we've been
shutdown (as can be seen by the fact that we have a null mDocument pointer); in
that case, it's problematic for us to be assigning 'mObservingState' here. The
mObservingState assignment is meant to be *undoing* the temporary assignments
earlier in the function, but in this post-Shutdown situation, it's actually
causing us to forget that we already unregistered as a refresh observer, in a
nested stack level's call to Shutdown().
I think this one place is the only mObservingState assignment that's got this
problem. The other mObservingState assignments should all be OK (they don't
need a "have we shut down" check), because they're all either:
a) modifying mObservingState to record an actual change that we just made to
our RefreshDriver registration,
...or:
b) immediately adjacent to code that dereferences mDocument, implying that
mDocument is known-to-be-non-null at that point, which means we know we haven't
been Shutdown.
This patch unblocks us from adding a release assert in the
NotificationController destructor in the next patch in this series. (Without
this patch, that assertion can be made to fail, via this inadvertent
mObservingState assignment.)
Depends on D195041
Differential Revision: https://phabricator.services.mozilla.com/D195200
If these assertions somehow fail, then we are probably doomed to crash anyway.
The new release assertions just make it so we'll crash in a controlled way,
with a more actionable backtrace, closer to the problem-spot.
This patch also removes one mDocument null-check that's becoming redundant
since it follows a release-assert that would make us abort if the pointer is
null.
Depends on D195041
Differential Revision: https://phabricator.services.mozilla.com/D195042
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 revision updates DropMutationEvent such that it handles other event
types besides REORDER, HIDE, and SHOW. If we somehow put a different event
type through this function (particularly an inner reorder, which indicates
a reorder event that's already been dropped), we should (and now do) handle
it more gracefully.
Differential Revision: https://phabricator.services.mozilla.com/D182218
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
If we move an accessible via an aria-owns relocation on a grandparent, then move
it again via an aria-owns relocation on a parent, we can end up with multiple
hide events in the queue: one for the shallowest hide (of the grandparent), and
one for the hide of the grandchild accessible. We can't always coalesce (drop)
the hide event of the grandchild since the first move severs the relationship
between its parent and its grandparent. To address this, this revision stops us
from queueing a hide event on an accessible that's already being moved. This
revision also adds a test to verify we're getting the proper events.
Differential Revision: https://phabricator.services.mozilla.com/D180888
We only needed to defer events because of the need to wait for a parent COM proxy.
Similarly, sync text change events (as opposed to async) were only necessary with the COM proxy architecture.
Now that there are no COM proxies, neither of these are necessary.
Differential Revision: https://phabricator.services.mozilla.com/D177899
This was done using the following script:
```
matchingFiles=`git grep -l accessibility_cache_enabled`
sed -i 's/StaticPrefs::accessibility_cache_enabled_AtStartup/a11y::IsCacheActive/' $matchingFiles
for f in $matchingFiles; do
usesA11yPref=
grep -q StaticPrefs::accessibility_ $f && usesA11yPref=1
hasA11yInclude=
grep -q 'nsAccessibilityService.h' $f && hasA11yInclude=1
if [ -z $usesA11yPref ] && [ -z $hasA11yInclude ]; then
sed -i 's,mozilla/StaticPrefs_accessibility.h,nsAccessibilityService.h,' $f
elif [ -z $usesA11yPref ] && [ $hasA11yInclude ]; then
sed -i '/"mozilla.StaticPrefs_accessibility.h"/d' $f
elif [ $usesA11yPref ] && [ -z $hasA11yInclude ]; then
sed -i '/"mozilla.StaticPrefs_accessibility.h"/a#include "nsAccessibilityService.h"' $f
fi
done
```
Differential Revision: https://phabricator.services.mozilla.com/D177722
James Teh