The changeset has been generated automatically through:
./mach static-analysis check --output ./clang-tidy-bugprone.json --format json --fix
followed by manual adjustment when needed or when the auto-fixer bailed out.
This paves the way for Bug 2023527
Differential Revision: https://phabricator.services.mozilla.com/D287941
The changeset has been generated automatically through:
./mach static-analysis check --output ./clang-tidy-bugprone.json --format json --fix
followed by manual adjustment when needed or when the auto-fixer bailed out.
This paves the way for Bug 2023527
Differential Revision: https://phabricator.services.mozilla.com/D287941
Firing a caret event for an Accessible which doesn't have focus confuses clients, who expect that the caret should be related to the focus.
Prior to bug 2006816, this wasn't as bad because even though we fired the caret event on the wrong Accessible, TextLeafPoint::GetCaret still returned the caret from the focused document.
After bug 2006816, we use the cached caret, so the event and the retrieved caret are both (consistently) incorrect.
To fix this:
1. Make it possible to update the RemoteAccessible cache for the caret via IPDL without firing a caret event.
2. When the caret moves, don't fire a caret event unless it's within the focused document.
3. However, the caret event won't be fired again when the document eventually gets focused, so we still send an IPDL message to update the cached caret in the RemoteAccessible tree.
4. In TextLeafPoint::GetCaret, only use the cached caret if the cached caret Accessible is within the same document as the origin Accessible.
Differential Revision: https://phabricator.services.mozilla.com/D282330
`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
This is now simply a matter of leveraging the work in the previous patches to support the new selection type and map it to the correct attribute.
There isn't an IAccessible2 or ATK attribute for this yet.
We use mark:true, based on <mark> and role="mark" which is used for a semantic highlight.
I've discussed this with Google, NV Access and Vispero and they all seem to be happy with this.
Differential Revision: https://phabricator.services.mozilla.com/D217071
nsISelectionListener isn't sufficient because it only notifies that the selection has changed.
A11y needs to know specifically which ranges were added or removed.
Previously, we handled this specifically for spelling errors in mozInlineSpellChecker.
That code has been removed and replaced with a more general approach.
Since we now have these notifications, rather than using nsISelectionListener just to fire events, we now use these notifications for that as well.
This avoids the need to add additional nsISelectionListeners, which would get messy particularly for custom highlights where there can be an arbitrary number of selections at any given time.
Differential Revision: https://phabricator.services.mozilla.com/D217068
This doesn't matter for spelling errors.
However, we will soon generalise this code for use with other attributes like this.
In particular, custom highlights require us to use AbstractRange.
While we aren't supporting custom highlights in this bug, it is more effective to do this as part of this major refactor rather than doing another refactor later.
Differential Revision: https://phabricator.services.mozilla.com/D217063
Previously, we assumed that CaretAssociationHint::Before always meant the caret was at the insertion point at the end of a line.
However, it can also mean that the caret is before the start of a node in the middle of a line.
To fix this, we need to check for line and paragraph boundaries.
See the code comments for details.
I moved this functionality completely out of HyperTextAccessible and into TextLeafPoint.
First, it was easiest to do these checks with private functions already available to TextLeafPoint.
Second, this ideally belongs in TextLeafPoint anyway; its existence in HyperTextAccessible was vestigial.
Differential Revision: https://phabricator.services.mozilla.com/D215089
Previously, we assumed that CaretAssociationHint::Before always meant the caret was at the insertion point at the end of a line.
However, it can also mean that the caret is before the start of a node in the middle of a line.
To fix this, we need to check for line and paragraph boundaries.
See the code comments for details.
I moved this functionality completely out of HyperTextAccessible and into TextLeafPoint.
First, it was easiest to do these checks with private functions already available to TextLeafPoint.
Second, this ideally belongs in TextLeafPoint anyway; its existence in HyperTextAccessible was vestigial.
Differential Revision: https://phabricator.services.mozilla.com/D215089
This revision removes unnecessary include directives from cpp files in the
accessible/base directory. These suggestions came from the Include What You Use
tool.
Differential Revision: https://phabricator.services.mozilla.com/D182288
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
This was wasting memory for the parent process, and when the cache was disabled, all content processes.
QueueCacheUpdate now checks whether we are actually sending cache updates before it queues anything.
Simple calls rely on this check, rather than guarding the call to QueueCacheUpdate themselves.
We still call QueueCacheUpdate conditionally for more complex cases which would have to do work before calling QueueCacheUpdate, since we don't want to pointlessly do that prerequisite work.
Differential Revision: https://phabricator.services.mozilla.com/D149231
I originally thought we didn't need to be explicitly notified about removals because a correction always triggers a text change and we push a cache update for spelling errors whenever the text changes.
However, it seems that even when a correction is made and the text changes, the spell check ranges aren't yet up to date.
Previously, we were pushing a cache update whenever text was reflowed, which was taking care of this.
Soon, we don't want to push text cache updates if the text doesn't actually change.
We remove the reliance on these redundant text updates by having an explicit notification for spell check removals.
Differential Revision: https://phabricator.services.mozilla.com/D147655
We already have an nsISelectionListener, but that only tells us that a change happened somewhere in the selection, not which range changed.
We don't want to push a cache update for all ranges when only one changed.
Therefore, this patch adds an accessibility notification in mozInlineSpellChecker::AddRange.
We don't need this for removed ranges because the text will change for any spelling error corrections and text updates trigger spelling error cache updates.
Differential Revision: https://phabricator.services.mozilla.com/D147244
As well as the caret offset, we also need to cache whether the caret is at the end of a line.
Therefore, this information has been added to caret events.
Differential Revision: https://phabricator.services.mozilla.com/D132097
This is a collaboration between clang-tidy and clang-format.
clang-tidy uses compiled code paths, so this patch is linux specific.
Differential Revision: https://phabricator.services.mozilla.com/D105669
The inclusions were removed with the following very crude script and the
resulting breakage was fixed up by hand. The manual fixups did either
revert the changes done by the script, replace a generic header with a more
specific one or replace a header with a forward declaration.
find . -name "*.idl" | grep -v web-platform | grep -v third_party | while read path; do
interfaces=$(grep "^\(class\|interface\).*:.*" "$path" | cut -d' ' -f2)
if [ -n "$interfaces" ]; then
if [[ "$interfaces" == *$'\n'* ]]; then
regexp="\("
for i in $interfaces; do regexp="$regexp$i\|"; done
regexp="${regexp%%\\\|}\)"
else
regexp="$interfaces"
fi
interface=$(basename "$path")
rg -l "#include.*${interface%%.idl}.h" . | while read path2; do
hits=$(grep -v "#include.*${interface%%.idl}.h" "$path2" | grep -c "$regexp" )
if [ $hits -eq 0 ]; then
echo "Removing ${interface} from ${path2}"
grep -v "#include.*${interface%%.idl}.h" "$path2" > "$path2".tmp
mv -f "$path2".tmp "$path2"
fi
done
fi
done
Differential Revision: https://phabricator.services.mozilla.com/D55443
This patch makes accessible module use `mozilla::PresShell` directly rather
than via `nsIPresShell`. Additionally, renames `DocAccessible::PresShell()`
to `DocAccessible::PresShellPtr()` for avoiding conflict with using
`PresShell` in it and its sub classes.
Differential Revision: https://phabricator.services.mozilla.com/D26663
Summary: Really sorry for the size of the patch. It's mostly automatic
s/nsIDocument/Document/ but I had to fix up in a bunch of places manually to
add the right namespacing and such.
Overall it's not a very interesting patch I think.
nsDocument.cpp turns into Document.cpp, nsIDocument.h into Document.h and
nsIDocumentInlines.h into DocumentInlines.h.
I also changed a bunch of nsCOMPtr usage to RefPtr, but not all of it.
While fixing up some of the bits I also removed some unneeded OwnerDoc() null
checks and such, but I didn't do anything riskier than that.
This SelectionManager assertion fails when running the testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py TestBackForwardNavigation.test_non_remote_about_pages marionette test. This assertion failure is bug 1126649.
This patch DOES NOT fix the cause of the assertion failure (a missing HyperTextAccessible). It just replaces this failing NS_NOTREACHED with NS_ERROR because I am removing the NS_NOTREACHED macro.
MozReview-Commit-ID: EVfyZlMwN4p
None of the C++ callers of RemoveSelectionListener care about whether the
listener was already-added, and the only JS caller is in a test and knows the
listener was added. So the behavior change to no-op instead of throwing when
trying to remove a nonexistent listener is OK. Furthermore, the removal is
null-safe, so there's no point to explicitly failing if null is passed (which
it never is).
Since content can't directly add selection listeners, we can just use an
infallible append instead of returning errors callers don't check for anyway.
Also, no one passes null to AddSelectionListener, so we don't have to worry
about that part.
Daniel Holbert