The nsAccUtils method was a "fast" one for calculating set sizes and
conceptual parents. Unified it with
LocalAccessible::GetLevelInternal in Accessible::GetLevel with an
argument.
I also fixed select->optgroup->option group attributes.
Differential Revision: https://phabricator.services.mozilla.com/D134208
While mingw builds don't require user32 and advapi32 explicitly, it doesn't
hurt for them to be there (and they're required for clang-cl build).
Likewise, while clang-builds don't require uuid and userenv explicitly
because they're pulled in via #pragmas in the source code, mingw doesn't
support those #pragmas and needs them explicitly, which doesn't hurt the
clang-cl builds.
Differential Revision: https://phabricator.services.mozilla.com/D134737
While mingw builds don't require user32 and advapi32 explicitly, it doesn't
hurt for them to be there (and they're required for clang-cl build).
Likewise, while clang-builds don't require uuid and userenv explicitly
because they're pulled in via #pragmas in the source code, mingw doesn't
support those #pragmas and needs them explicitly, which doesn't hurt the
clang-cl builds.
Differential Revision: https://phabricator.services.mozilla.com/D134737
Our default traversal rule will eventually work with remote trees, but
explore by touch will be local-only until we do hittesting in the parent
process. This change prevents the parent process from drilling down past
outer docs into frames.
Differential Revision: https://phabricator.services.mozilla.com/D132841
A move is sent to the parent process as a hide and then a show, even if the LocalAccessible wasn't recreated.
Previously, this meant that a new RemoteAccessible was created and the original RemoteAccessible was destroyed.
This was particularly problematic if the Accessible had focus and the client cached focus.
In that case, the client cache contained a dead Accessible, and since no focus event was fired (because it's the same Accessible in the content process), focus in the client was broken.
This also meant that there was a full cache push for a moved Accessible, which was wasteful.
To fix this:
1. Keep track of LocalAccessibles inserted during a tick.
2. Keep track of LocalAccessibles moved during a tick, including descendants. If a LocalAccessible was inserted during the same tick (1), don't track it as a move.
3. Before processing mutation events, tell the parent process about which Accessibles are about to be moved (2).
4. When sending a subtree to the parent process, don't send cache info for an Accessible which is being moved (2).
5. When the parent process receives a hide event, if there is an Accessible that is about to be moved (3), don't shut it down; allow it to be reused.
6. When the parent process receives a show event, if there is an Accessible that has an existing RemoteAccessible (5), reuse it.
Differential Revision: https://phabricator.services.mozilla.com/D132328
Some callers of `nsINode::ComputeIndexOf()` do not use its parent node except
calling it. These tiny methods make such callers simpler.
Differential Revision: https://phabricator.services.mozilla.com/D131337
This patch fixes only the cases if the result of `ComputeIndexOf_Deprecated()`
is used as unsigned integer with implicit or explicit cast.
Differential Revision: https://phabricator.services.mozilla.com/D131336
It's hard to fix some callers. Therefore, in this bug, we should fix only
simple cases. Therefore, we should rename existing API first.
Differential Revision: https://phabricator.services.mozilla.com/D131334
It takes text node's offset that is never larger than `INT32_T` because
`nsTextFragment` limits max text length of a text node is
`<= NS_MAX_TEXT_FRAGMENT_LENGTH`, that means equals or less than `0x1FFFFFFF`.
So that it must be safe to convert the offset to `uint32_t` from `int32_t`.
Differential Revision: https://phabricator.services.mozilla.com/D131111
They are defined as "unsigned long" by the standards. So we should use
`uint32_t` rather than `int32_t` with the methods. However, layout code
uses `int32_t` a lot for representing the offset. Therefore, this patch
adds `*_FixOffset1` etc for the cases which cannot fix easily in this patch.
Differential Revision: https://phabricator.services.mozilla.com/D131110
It's an internal API corresponding to `Selection.getRangeAt` DOM API.
I think that it should use `uint32_t` rather than `size_t` because of the
consistency with the DOM API and `Selection::RangeCount()`.
This patch fixes all callers of `GetRangeAt()`, and rewrites it with ranged-
loops unless original ones do not refer `RangeCount()` every time and may run
script in the loop.
Differential Revision: https://phabricator.services.mozilla.com/D128848
Building with --disable-xul has been busted since _at least_ bug
1082579, for more than 7 years (I didn't try to track that down
further). It's time to recognize that the option serves no purpose.
Differential Revision: https://phabricator.services.mozilla.com/D133161
I removed the sync IPDL implementation of LinkIndexOf because we can calculate this in the parent process even with the cache disabled.
Differential Revision: https://phabricator.services.mozilla.com/D132200
1. There was a Windows non-cached RemoteAccessible implementation, but it was never actually called, so I removed it.
2. The sync IPDL RemoteAccessible implementation previously provided a boolean indicating success.
I removed this because the LocalAccessible implementation doesn't have this and it doesn't seem like remote is special in this respect.
Differential Revision: https://phabricator.services.mozilla.com/D132198
-Wshadow warnings are not enabled globally, so these -Wno-shadow suppressions have no effect. I had intended to enable -Wshadow globally along with these suppressions in some directories (in bug 1272513), but that was blocked by other issues.
There are too many -Wshadow warnings (now over 2000) to realistically fix them all. We should remove all these unnecessary -Wno-shadow flags cluttering many moz.build files.
Differential Revision: https://phabricator.services.mozilla.com/D132289
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
The caret can be at the end of a wrapped line, but this does not have its own offset.
In this case, the way we handle the caret depends on what is being queried.
For character, we want to return no character at the position of the caret.
For word and line, we want to return the word/line before the caret.
To facilitate this conditional treatment, we use a special TextLeafPoint which represents the caret.
This is then actualized to a real point depending on the method called.
Differential Revision: https://phabricator.services.mozilla.com/D132095
1. When calculating a start offset, we usually want 0 if the offset can't be transformed, thus clipping to the start of this Accessible.
Previously, CharacterCount was always returned even when calculating a start offset.
Now, we return 0 for a start offset, CharacterCount for an end offset.
2. Sometimes, this fallback value is insufficient and we need to know explicitly whether it failed.
As well as the offset, we now return a boolean indicating this.
This will be used in a subsequent patch.
3. Even though TransformOffset only deals with positive numbers, callers usually deal with signed numbers.
Therefore, we now use int32_t insteaed of uint32_t to avoid a lot of casting.
Differential Revision: https://phabricator.services.mozilla.com/D132094
Previously, we tried to get a COM proxy even when the cache was enabled, which just crashed.
As part of this, use a RefPtr instead of manually managing references.
Differential Revision: https://phabricator.services.mozilla.com/D132104
XUL images use nsImageBoxFrame, not nsImageFrame.
AccessibleType() returns eNoType on nsImageBoxFrame.
Changing this to return eImageType seems a bit risky and adding a new type just for an assertion seems pointless.
Therefore, we just check the frame type.
Differential Revision: https://phabricator.services.mozilla.com/D132051
Add nsIAutoCompleteResult.isRemovableAt method, to indicate whether the value
is removable, and do nothing for Shift+Delete on the item when the value is
not removable.
Depends on D131740
Differential Revision: https://phabricator.services.mozilla.com/D131741
1. Use dom events in RootAccessible to fire ACTIVE state changes.
2. Add DOMMenuItemInactive events to nsListControlFrame and fire it on
the previously active option.
3. Don't fire those DOM events on collapsed combo boxes.
4. Add ACTIVE state change events for collapsed combo box options.
Differential Revision: https://phabricator.services.mozilla.com/D130298
Have ARIAActiveDescendantIDMaybeMoved call ARIAActiveDescendantChanged regardless of
widget focus state. The latter already checks if the widget is focused. This gives us
a chance to fire and ACTIVE state change regardless of focus state.
If an id or aria-activedescendant attribute is about to change, fire an
ACTIVE state change on the previously active accessible.
Differential Revision: https://phabricator.services.mozilla.com/D130296
By allowing async DOM events to be dispatched, and advancing the refresh
driver we can force some calls to NotificationController::WillRefresh
and flush out any pending notifications or events.
This lets us more reliably catch unexpected events without the need for
a sentinel event at the end.
Differential Revision: https://phabricator.services.mozilla.com/D130295
1. Use dom events in RootAccessible to fire ACTIVE state changes.
2. Add DOMMenuItemInactive events to nsListControlFrame and fire it on
the previously active option.
3. Don't fire those DOM events on collapsed combo boxes.
4. Add ACTIVE state change events for collapsed combo box options.
Differential Revision: https://phabricator.services.mozilla.com/D130298
Have ARIAActiveDescendantIDMaybeMoved call ARIAActiveDescendantChanged regardless of
widget focus state. The latter already checks if the widget is focused. This gives us
a chance to fire and ACTIVE state change regardless of focus state.
If an id or aria-activedescendant attribute is about to change, fire an
ACTIVE state change on the previously active accessible.
Differential Revision: https://phabricator.services.mozilla.com/D130296
By allowing async DOM events to be dispatched, and advancing the refresh
driver we can force some calls to NotificationController::WillRefresh
and flush out any pending notifications or events.
This lets us more reliably catch unexpected events without the need for
a sentinel event at the end.
Differential Revision: https://phabricator.services.mozilla.com/D130295
With the rewrite, we reduce the dependency of lwbrk LineBreaker::Next(),
WordBreaker::Next(), and their `int32_t` sentinel values
NS_LINEBREAKER_NEED_MORE_TEXT and NS_WORDBREAKER_NEED_MORE_TEXT.
Differential Revision: https://phabricator.services.mozilla.com/D130392
`profiler_thread_is_being_profiled` is used a lot for markers, so it makes sense to have a specialized version, which is a bit shorter, and lives in ProfilerMarkers.h.
Differential Revision: https://phabricator.services.mozilla.com/D130009
With the rewrite, we reduce the dependency of lwbrk LineBreaker::Next(),
WordBreaker::Next(), and their `int32_t` sentinel values
NS_LINEBREAKER_NEED_MORE_TEXT and NS_WORDBREAKER_NEED_MORE_TEXT.
Differential Revision: https://phabricator.services.mozilla.com/D130392
`profiler_thread_is_being_profiled` is used a lot for markers, so it makes sense to have a specialized version, which is a bit shorter, and lives in ProfilerMarkers.h.
Differential Revision: https://phabricator.services.mozilla.com/D130009
1. Add TakeFocus in Windows PDocAccessible.
2. Move RecvTakeFocus into DocAccessibleChildBase so it is used for all platforms.
3. Move TakeFocus into the Accessible base class.
4. Move the IPDL version of TakeFocus into RemoteAccessibleBase so it is used for all plaforms.
5. Make the Windows RemoteAccessible::TakeFocus use the base (IPDL) implementation if the cache is enabled or COM otherwise.
6. Use Accessible::TakeFocus for ATK, Mac, Windows and XPCOM.
Differential Revision: https://phabricator.services.mozilla.com/D130327
For example, when disabling an HTML input, we get an event for SUPPORTS_AUTOCOMPLETION among others.
If we continue with the test after that state change and there hasn't been an ENABLED/UNAVAILABLE state change, the cache won't be up to date.
Unfortunately, we can't use untilCacheOk here because we don't do a cache push for state changes.
Differential Revision: https://phabricator.services.mozilla.com/D129643
This patch includes XPCOM support.
Unfortunately, LocalAccessible::State isn't const, so the base method can't be const for now either.
Differential Revision: https://phabricator.services.mozilla.com/D129640
We only expose this state on HTMLButtonAccessible, but we sometimes get these notifications for other controls; e.g. checkbox inputs.
We shouldn't fire a state change event for a state we never expose.
Differential Revision: https://phabricator.services.mozilla.com/D129673
Currently, checking whether an `EventTarget` is `nsINode` (or its concrete
classes) or not requires a QI, but it's expensive and used a lot while we
handle each event. Therefore, it'd be nicer for creating a virtual method,
`EventTarget::IsNode()` and use it for the check.
If trying to convert `EventTarget` to a concrete class, it may require two
virtual method calls. I'm not sure whether it's cheaper than a QI, but at
least, it won't depend on the UUID check order of `QueryInterface()` when
multiple interfaces are implemented.
Differential Revision: https://phabricator.services.mozilla.com/D129781
Since this is a chrome page we start honoring dark mode colors by
default, and after bug 1525107 that affects default background and text
colors, which these tests were hardcoding to black/white.
Differential Revision: https://phabricator.services.mozilla.com/D129983
This is necessary for two reasons:
1. Leaf text attributes and default text attributes are cached separately, since the caller can choose whether to include defaults or not. This means merging them together.
2. Methods for retrieving attributes currently return a non-const AccAttributes. We might be able to change this in future for some of them, but I didn't want to deal with that in this work.
Differential Revision: https://phabricator.services.mozilla.com/D129472
This uses TextLeafPoint.
This patch includes support for ATK, IA2 and XPCOM.
As with TextAtOffset, HyperTextAccessible calls the base implementation if the cache is enabled, but otherwise uses the old implementation for now.
Differential Revision: https://phabricator.services.mozilla.com/D129469
This supports both fetching text attributes and finding the start of attribute runs, but only for LocalAccessible.
Support for RemoteAccessible will be added in a subsequent patch.
Differential Revision: https://phabricator.services.mozilla.com/D129468
This will be used later to check whether text attributes are different between two Accessibles.
I added this as a method instead of operator== because callers always hold a pointer to AccAttributes, and *a1 == *a2 is weird and will probably lead to mistakes.
This required adding operator== for our value structs.
Differential Revision: https://phabricator.services.mozilla.com/D129467
I think this patch is functionally neutral.
I'm not completely sure about the best way to do these casts - the __bridge may not be
necessary since we don't use ARC yet. But it's probably fine to add it anyway.
Also, returning autoreleased CFTypeRef objects seems a bit weird, but it's what we've
already been doing and it's probably fine.
And some of these nils should maybe be nullptrs, but the compiler doesn't seem to care.
Differential Revision: https://phabricator.services.mozilla.com/D129559
mCachedFields might be null because all fields are default; i.e. mCachedFields being null doesn't mean the cache is invalid/disabled.
We should check the pref instead when determining whether to call base class methods to query the cache.
Differential Revision: https://phabricator.services.mozilla.com/D128445
IsStartOfGroup wants to peek at the previous character.
It calls PrevChar, but it doesn't check for failure.
It then unconditionally increments mOffset to "undo" PrevChar.
If PrevChar failed, that means we're now 1 character after where we started.
This wasn't causing any test failures and I can't think of a test case that shows breakage here.
Nevertheless, it definitely doesn't make sense and could cause difficult-to-diagnose bugs.
This in turn exposed a bug in PrevChar whereby it didn't handle being called again after it had already failed.
That also had to be fixed here.
Differential Revision: https://phabricator.services.mozilla.com/D128332
Trickier than just listening for attributes on inputs since a form
can toggle autocomplete too.
Also fixed To32States to work on the 32nd bit.
Differential Revision: https://phabricator.services.mozilla.com/D128913
1. Don't return an error from IAccessible2::states. Otherwise, NVDA assumes the Accessible is defunct.
2. Return STATE_SYSTEM_READONLY for all documents in IAccessible::get_accState. Otherwise, NVDA doesn't treat the document as browseable.
Differential Revision: https://phabricator.services.mozilla.com/D128556
This makes way for a new unified TextAcc method in a subsequent patch.
Strictly speaking, doing it like this isn't correct because methods will return CO_E_OBJNOTCONNECTED for a RemoteAccessible instead of E_NOTIMPL.
I fixed this for IAccessibleText, but I realised fixing this for all interfaces is going to involve a lot of churn which we're going to end up removing anyway once we unify everything.
Clients don't really care about the error code, so I think this situation is acceptable while this is still in early development.
Differential Revision: https://phabricator.services.mozilla.com/D128554
We don't support any of the methods in this interface for RemoteAccessible yet.
However, IAccessibleHypertext::get_hyperlink returns an IAccessibleHyperlink, so we must support QI to it.
Differential Revision: https://phabricator.services.mozilla.com/D128553
This uses the unified Accessible::EmbeddedChildAt from the previous patch.
HyperTextAccessible::LinkAt still exists, since there are still some callers that depend on it returning a LocalAccessible.
xpcAccessibleHyperText has also been updated to call this, which means tests will work on Windows when the cache is enabled.
Differential Revision: https://phabricator.services.mozilla.com/D128551
This required renaming LocalAccessible::GetEmbeddedChildAt to EmbeddedChildAt.
LocalAccessible::EmbeddedChildAt uses a covariant return type to return a LocalAccessible* while still overriding the base method (which returns Accessible*).
Unfortunately, this can't be done for RemoteAccessible, since the Derived type isn't complete in time due to the templating.
There was only one caller of RemoteAccessible::EmbeddedChildAt, so I just changed it to call AsRemote.
Eventually, even that caller will use the unified tree anyway.
Differential Revision: https://phabricator.services.mozilla.com/D128550
This can happen for display: contents.
In that case, there are no lines in the Accessible anyway, so returning an invalid TextLeafPoint (as we already did) is the correct thing to do.
Differential Revision: https://phabricator.services.mozilla.com/D128319
When the cache is disabled, we need a different implementation of RemoteAccessible::CharacterCount for non-Windows.
This already existed, but it hid the base implementation rather than overriding it.
On Windows, we don't support this for RemoteAccessible at all unless the cache is enabled, so there's no change there.
Differential Revision: https://phabricator.services.mozilla.com/D128031
This is a follow-up to my original patch.
It turns out that there is an assertion preventing use of document.write in privileged contexts which my original patch triggered.
Instead, set document.body.innerHTML.
Differential Revision: https://phabricator.services.mozilla.com/D128159
If a select's size is greater than 1 it won't be recreated when multiple
is added or removed. State change events should be fired in that case.
Differential Revision: https://phabricator.services.mozilla.com/D128277
If a <select> element with a default size or size of 1 gets the
`multiple` attribute it will change from a combobox button to an
embedded listbox. Similarly, if a select's size is set to something
larger than 1 it will convert from a combobox button to a listbox.
We should recreate the select's subtree because the
implementing classes are different for those two cases.
Differential Revision: https://phabricator.services.mozilla.com/D128276
We were force recreating accessibles when their aria-multiselectable
attribute changed. This was perhaps done in the past because of
percieved limitations in COM, or the implementing class was different.
This isn't the case anymore. We should instead fire state change events
when aria-multiselectable changes.
Differential Revision: https://phabricator.services.mozilla.com/D128275
Use imgINotificationObserver to get notified of size availability.
Layout uses an observer to wait for this too. Our observer is notified
after layout so we should have bounds by then.
We need to store the request status because we get a lot of "replayed"
status changes that would cause chatty and wrong state change events
when the accessible is first created and bound to parent.
We can use that status for both INVISIBLE and ANIMATED states in
NativeState.
Differential Revision: https://phabricator.services.mozilla.com/D127719
There are a lot of check of `Document`'s editable state **with** comments. This
means that it's unclear for developers that only `Document` node is editable in
design mode.
Additionally, there are some points which use composed document rather than
uncomposed document even though the raw API uses uncomposed document. Comparing
with the other browsers, checking uncomposed document is compatible behavior,
i.e., nodes in shadow trees are not editable unless `contenteditable`.
Therefore, `nsINode` should have a method to check whether it's in design mode
or not.
Note that it may be called with a node in UA widget. Therefore, this patch
adds new checks if it's in UA widget subtree or native anonymous subtree,
checking whether it's in design mode with its host.
Differential Revision: https://phabricator.services.mozilla.com/D126764
Use imgINotificationObserver to get notified of size availability.
Layout uses an observer to wait for this too. Our observer is notified
after layout so we should have bounds by then.
We need to store the request status because we get a lot of "replayed"
status changes that would cause chatty and wrong state change events
when the accessible is first created and bound to parent.
We can use that status for both INVISIBLE and ANIMATED states in
NativeState.
Differential Revision: https://phabricator.services.mozilla.com/D127719
Use imgINotificationObserver to get notified of size availability.
Layout uses an observer to wait for this too. Our observer is notified
after layout so we should have bounds by then.
We need to store the request status because we get a lot of "replayed"
status changes that would cause chatty and wrong state change events
when the accessible is first created and bound to parent.
We can use that status for both INVISIBLE and ANIMATED states in
NativeState.
Differential Revision: https://phabricator.services.mozilla.com/D127719
Normally, we use data: URLs to load snippets as tab documents.
However, there is no way we can make this load in the parent process.
Using a chrome:// URL will allow us to load a file in the parent process.
For OOP iframes, we use document-builder.sjs to yield a document with supplied markup, but sjs doesn't work for chrome:// (since sjs is handled by our test web server).
Instead, for chrome://, we use an HTML file with an embedded script which replaces the content of the document with the supplied markup.
Differential Revision: https://phabricator.services.mozilla.com/D127769
1. Use ia2AccessibleHypertext for RemoteAccessibles that support hypertext.
2. Allow querying to the IAccessibleText interface for RemoteAccessibles. IAccessibleHypertext, etc. are not yet supported at all, so don't allow querying to those.
3. Use HyperTextAccessibleBase for methods it supports. For other IAccessibleText methods, return E_NOTIMPL if dealing with a RemoteAccessible.
Differential Revision: https://phabricator.services.mozilla.com/D127217
The TextLeafPoint implementation works for both local and remote.
HyperTextAccessible calls the base implementation for supported boundaries when the cache is enabled.
Differential Revision: https://phabricator.services.mozilla.com/D127215
This is pretty similar to HyperTextAccessible::TransformOffset.
However, the local implementation has to compensate for list bullet errors propagating from FindOffset/PeekOffset, which isn't relevant for TextLeafPoint.
Therefore, we leave the HyperTextAccessible implementation alone so it will still be used by HyperTextAccessible code (FindOffset, etc.).
Differential Revision: https://phabricator.services.mozilla.com/D127214
The implementation for RemoteAccessibleBase is just a stub; we don't cache the caret yet.
However, this needs to be in HyperTextAccessibleBase because it is used by ConvertMagicOffset, which will be moved in an upcoming patch.
Differential Revision: https://phabricator.services.mozilla.com/D127209
Although HyperTextAccessibleBase implements these, we leave the HyperTextAccessible implementations (overriding the base) because they do caching.
Depending on performance, we may eventually want to move the caching into the base implementation.
I decided not to do this initially because it will use extra memory in the parent process and it may be a premature optimisation.
Differential Revision: https://phabricator.services.mozilla.com/D127207
This will contain methods that can be used for both local and remote Accessibles.
It is inherited into both HyperTextAccessible (local) and RemoteAccessibleBase.
Differential Revision: https://phabricator.services.mozilla.com/D127206
The TextLeafPoint implementation works for both local and remote.
HyperTextAccessible calls the base implementation for supported boundaries when the cache is enabled.
Differential Revision: https://phabricator.services.mozilla.com/D127215
This is pretty similar to HyperTextAccessible::TransformOffset.
However, the local implementation has to compensate for list bullet errors propagating from FindOffset/PeekOffset, which isn't relevant for TextLeafPoint.
Therefore, we leave the HyperTextAccessible implementation alone so it will still be used by HyperTextAccessible code (FindOffset, etc.).
Differential Revision: https://phabricator.services.mozilla.com/D127214
The implementation for RemoteAccessibleBase is just a stub; we don't cache the caret yet.
However, this needs to be in HyperTextAccessibleBase because it is used by ConvertMagicOffset, which will be moved in an upcoming patch.
Differential Revision: https://phabricator.services.mozilla.com/D127209
Although HyperTextAccessibleBase implements these, we leave the HyperTextAccessible implementations (overriding the base) because they do caching.
Depending on performance, we may eventually want to move the caching into the base implementation.
I decided not to do this initially because it will use extra memory in the parent process and it may be a premature optimisation.
Differential Revision: https://phabricator.services.mozilla.com/D127207
This will contain methods that can be used for both local and remote Accessibles.
It is inherited into both HyperTextAccessible (local) and RemoteAccessibleBase.
Differential Revision: https://phabricator.services.mozilla.com/D127206
As part of this, we no longer cache the name for text accessibles, as that would be redundant.
Instead, we return the cached text in RemoteAccessible::Name.
Differential Revision: https://phabricator.services.mozilla.com/D127202
This patch shouldn't change behavior at all.
This patch replaces a manual NS_ADDREF call with typesafe code that manages the
reference count for us. This reduces repeated boilerplate code, in the
implementation as well as the callsites.
Differential Revision: https://phabricator.services.mozilla.com/D127179
Also, make the same change to nsPresContext::GetTextInputHandlingWidget and
TextComposition::GetWidget, which are essentially aliases/wrappers for this
function.
This patch shouldn't change behavior at all, aside from:
* optimizing away some redundant reference counting and widget-lookups
* delaying some nsIWidget::Release() calls, which will now happen after we're
actually done using the object, instead of happening when the getter
completes. (It's unlikely this impacts behavior, because there are other
objects that are keeping the nsIWidget instance alive.)
Motivation / "wins" from this patch:
* nsPresContext::GetRootWidget already works with a refcounted pointer
internally. Before this patch, it drops the reference before returning the
pointer. This is a bit suspect and would cause security issues, in the
unlikely event that this were the last strong reference to the object. It
can just as easily/efficiently transfer the strong reference to the caller,
and let the caller determine when to release it.
* Many of the callers were already storing the return value in nsCOMPtr, which
meant that they were incurring an additional AddRef/Release when
populating/destructing that smart pointer. Now they can just take ownership
of the already_AddRefed return value and avoid redundnat refcount-churn.
* For the callers that weren't storing the return value in nsCOMPtr, some of
them were calling this getter twice in a row (once to test for truthiness and
once to use the known-truthy value). This was wasteful, both from the
repeated lookup-work (since the function isn't a trivial getter), and from
repeated refcount-churn. This patch collapses these repeat-calls to a single
call, avoiding those inefficiencies.
Differential Revision: https://phabricator.services.mozilla.com/D127180
This patch shouldn't change behavior at all.
This patch replaces a manual NS_ADDREF call with typesafe code that manages the
reference count for us. This reduces repeated boilerplate code, in the
implementation as well as the callsites.
Differential Revision: https://phabricator.services.mozilla.com/D127179
Before this patch, we didn't set aria-valuetext when the value was initially empty, nor did we clear it after a value was set and subsequently cleared.
This resulted in incorrect numeric values being reported to users.
Now, we set aria-valuetext to "" in either of these cases.
Differential Revision: https://phabricator.services.mozilla.com/D125366
Before this patch, we didn't set aria-valuetext when the value was initially empty, nor did we clear it after a value was set and subsequently cleared.
This resulted in incorrect numeric values being reported to users.
Now, we set aria-valuetext to "" in either of these cases.
Differential Revision: https://phabricator.services.mozilla.com/D125366
This passes for me on try.
We do have some low frequency intermittent failures for this test, but they apply to all platforms.
Differential Revision: https://phabricator.services.mozilla.com/D126428
This means nsBlockFrame::GetLineIterator is also infallible, so callers that know
they're dealing with an nsBlockFrame (not an arbitrary nsIFrame) don't need to
check for null.
Differential Revision: https://phabricator.services.mozilla.com/D126470
Changed the array type to nsTArray to avoid copies and get compile-time
errors if we ever try to do that. To set an array as a value, it must be
moved.
Differential Revision: https://phabricator.services.mozilla.com/D125899
Previously, the test asserted that the line offsets for an embedded object should be an empty range.
This is actually incorrect, since there is content on the line (an empty text box and a button).
Update this with the correct result, but mark it as an expected failure.
The new implementation fixes this, so once we enable it by default, we can assert success.
Differential Revision: https://phabricator.services.mozilla.com/D125670
We won't use HyperTextAccessible for building the cache.
However, we have a lot of existing single process mochitests for HyperTextAccessible.
Putting it behind this pref makes it easy for us to run those tests against this new text implementation.
Eventually, we want to use this implementation for both local and remote Accessibles.
Differential Revision: https://phabricator.services.mozilla.com/D124778
This implementation uses WordBreaker and a bunch of code to emulate layout's handling of punctuation, space, words spanning across nodes, words broken due to lines, etc.
While this does mean we're effectively duplicating layout, there are a couple of advantages.
First, PeekOffset has a lot of quirks which have caused us problems in the past and are difficult to debug.
They don't matter so much for other consumers, but a11y is very sensitive to these kinds of bugs.
Having our own logic means we can make changes as needed without potentially affecting other PeekOffset consumers.
Second, while the implementation currently only works on LocalAccessibles (because we don't have text in RemoteAccessible yet), it's been designed so that it should be very easy to adapt for RemoteAccessible.
It already walks the unified Accessible tree, so it just has to be taught how to get text from RemoteAccessible once that's possible.
This means we don't have to cache word boundaries; they can be calculated on demand in the parent process.
Differential Revision: https://phabricator.services.mozilla.com/D124775
This adds TextLeafPoint::FindBoundary, which will be the main entry point for finding single boundaries.
FindBoundary searches individual Accessibles for a boundary, walking the a11y tree and continuing the search if a boundary isn't found.
Support for line start boundaries uses layout to determine line edges, but otherwise traverses the accessibility tree.
This avoids depending on PeekOffset, which has a lot of quirks that cause problems for a11y.
More importantly, it's possible to find line boundaries within a single LocalAccessible using the lower level FindPrev/NextLineStartSameLocalAcc methods.
This will be useful for line boundary caching, since we want to cache this info on individual Accessibles.
In contrast, PeekOffset might walk outside the current Accessible, which makes caching more difficult.
Differential Revision: https://phabricator.services.mozilla.com/D124774
accessible/base/TreeWalker.cpp:151:10: error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
accessible/xpcom/xpcAccessibleMacInterface.mm:177:10: error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
Differential Revision: https://phabricator.services.mozilla.com/D126173
Changed the array type to nsTArray to avoid copies and get compile-time
errors if we ever try to do that. To set an array as a value, it must be
moved.
Differential Revision: https://phabricator.services.mozilla.com/D125899
This allows us to support 255 content processes instead of 127.
It also means we have 1 less bit for the Accessible portion of the id, which means we can support less Accessibles per process.
Hopefully, this shouldn't be a problem, especially with Fission generally meaning less documents per process.
Differential Revision: https://phabricator.services.mozilla.com/D104344
There can be a scenario where an initial cache is pushed to an accessible via an update and not an "initial" push that a doc load or a subtree show would give.
For example, an accessible might not have a name, description, or numeric value (that is all we currently cache), but then get a name later in its lifetime. If that is the case the accessible will get a cache AccAttributes with a DeleteEntry value for "description" since its description is still empty. That entry should not be stored in the cache.
Differential Revision: https://phabricator.services.mozilla.com/D124484
By using a real image, and not a broken one, we avoid the recreation
that happens when the image gains and loses an alt. We can then rely on
name/description changes.
This is a more accurate test that doesn't rely on tree mutations.
Differential Revision: https://phabricator.services.mozilla.com/D124233
When constructing the initial cache, don't push empty or default fields.
Also, have a way to remove fields from an established cache if it
becomes empty/default.
Differential Revision: https://phabricator.services.mozilla.com/D124127
Automatically generated path that adds flag `REQUIRES_UNIFIED_BUILD = True` to `moz.build`
when the module governed by the build config file is not buildable outside on the unified environment.
This needs to be done in order to have a hybrid build system that adds the possibility of combing
unified build components with ones that are built outside of the unified eco system.
Differential Revision: https://phabricator.services.mozilla.com/D122345