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
The summary class just had a moxExpanded which we already have in mozAccessible.
I think the former was added before the latter was generalized for aria-expanded usage.
Differential Revision: https://phabricator.services.mozilla.com/D123516
Eventually we will want to add the Name method as an abstract method in
Accessible and implemented it platform-independent in
RemoteAccessibleBase.
Differential Revision: https://phabricator.services.mozilla.com/D121925
This is a good place to formalize the following naming convention:
* A "field" is and direct accessible getter method (name,
role, value, min, max, etc.)
* An "attribute" is a member of the "attributes" field.
With that said, I think AccAttributes should probably be named
AccProperties or something of the sort. Might leave that for another
time.
Differential Revision: https://phabricator.services.mozilla.com/D121924
This will allow us to use the list of accessibles to be serialized later
for pushing a cache. The single array also gives us an opportunity to
paginate the cache, if needed.
Differential Revision: https://phabricator.services.mozilla.com/D121923
As part of this, I changed RemoteAccessible::RemoteChildAt (and thus RemoteAccessible::ChildAt) so it doesn't crash when passed an invalid child index.
Our EnumVariant implementation relied on this with LocalChildAt.
I think it makes sense for LocalChildAt, RemoteChildAt and ChildAt to be consistent in this regard.
Differential Revision: https://phabricator.services.mozilla.com/D122681
Prior to this patch, we reported that we supported IEnumVARIANT, but it wouldn't return any children, causing the oleacc AccessibleChildren function to report no children to clients.
This is tricky to support due to the special COM proxy stuff we have to do for remote children without the cache.
There's no benefit to IEnumVARIANT with only one child anyway, so it's easiest to just say we don't support it.
The oleacc AccessibleChildren function will detect this and fall back to IAccessible::get_accChild instead.
I also took the opportunity to fix a slightly related outdated comment I spotted while figuring out the fix here.
Differential Revision: https://phabricator.services.mozilla.com/D122674
Firefox shows a blank window and then swaps it for the real thing for
percieved startup performance. This causes us to throw away the
activated state stored on the initial root widget. Instead of storing
the state, we should retrieve it from widget.
Differential Revision: https://phabricator.services.mozilla.com/D122168
Firefox shows a blank window and then swaps it for the real thing for
percieved startup performance. This causes us to throw away the
activated state stored on the initial root widget. Instead of storing
the state, we should retrieve it from widget.
Differential Revision: https://phabricator.services.mozilla.com/D122168
This will make implementing the new behavior behind a pref
really straight-forward, and is generally nicer.
Depends on D121858
Differential Revision: https://phabricator.services.mozilla.com/D121705
Changed the browser and mochitest name tests to rely exclusively on name change
events. To make this happen, I fixed all the cases where we were
event-deficient in the code:
* Examine target in PushNameOrDescriptionChange if it has eNameFromSubtreeRule.
Fixes cases where a text change event happens with the subtree name root as target.
* Change in aria-labelledby should always result in a name change event because
that attribute has highest prescedence.
* Add eHasNameDependent/eHasDescriptionDependent context flags when dependee accessible
is added after dependent accessible to tree.
* Handle value attribute change in HTML buttons and determine if they should trigger a
name changed event.
* Use accessible tree instead of content tree when calculating HTMLSelectOptionAccessible
name, this keeps the PushNameOrDescriptionChange sees in name flags consistent with
the actual tree.
* Handle label attribute change in select options and determine if they should trigger
a name changed event.
* Determine if s summary attribute change on a table triggers a name change event.
* If a title attribute is changed, reliably fire a name change event if
it is used in name calculation.
Differential Revision: https://phabricator.services.mozilla.com/D121580
Needed to tweak tests a bit because the code is now more descerning
wheter to fire an event or not. Will do so only if the state actually
changes.
Depends on D120901
Differential Revision: https://phabricator.services.mozilla.com/D120902
Also folded ARIAAttributeChanged into DOMAttributeChanged temporarily.
Will streamline that further in the next patch where state changes are
done a bit better.
Depends on D120900
Differential Revision: https://phabricator.services.mozilla.com/D120901
Needed to tweak tests a bit because the code is now more descerning
wheter to fire an event or not. Will do so only if the state actually
changes.
Depends on D120901
Differential Revision: https://phabricator.services.mozilla.com/D120902
Also folded ARIAAttributeChanged into DOMAttributeChanged temporarily.
Will streamline that further in the next patch where state changes are
done a bit better.
Depends on D120900
Differential Revision: https://phabricator.services.mozilla.com/D120901
Needed to tweak tests a bit because the code is now more descerning
wheter to fire an event or not. Will do so only if the state actually
changes.
Depends on D120901
Differential Revision: https://phabricator.services.mozilla.com/D120902
Also folded ARIAAttributeChanged into DOMAttributeChanged temporarily.
Will streamline that further in the next patch where state changes are
done a bit better.
Depends on D120900
Differential Revision: https://phabricator.services.mozilla.com/D120901
Previously, we used GetFlattenedTreeParent from the list box to find the autocomplete popup.
After bug 1708735, this now returns a slot instead of the panel.
We now use GetParentElement instead, which works as expected and is consistent with other code in this class anyway.
I also added a new test so this doesn't regress yet again.
We already have test_focus_autocomplete.xhtml which is supposed to test this, but that test is broken, was thus disabled and is complicated enough that I don't think we're going to fix it any time soon, if ever.
The new test was triggering an assertion on Windows when trying to handle a caret event, so HyperTextAccessible::GetCaretRect had to be tweaked slightly to fix this.
Differential Revision: https://phabricator.services.mozilla.com/D121163
Needed to tweak tests a bit because the code is now more descerning
wheter to fire an event or not. Will do so only if the state actually
changes.
Depends on D120901
Differential Revision: https://phabricator.services.mozilla.com/D120902
Also folded ARIAAttributeChanged into DOMAttributeChanged temporarily.
Will streamline that further in the next patch where state changes are
done a bit better.
Depends on D120900
Differential Revision: https://phabricator.services.mozilla.com/D120901
If an iframe is hidden, its OuterDocAccessible will be destroyed, but the PBrowserBridge still exists.
Since the OuterDocAccessible was destroyed, its id can be reused.
Therefore, if a new embedder accessible hasn't already been set, clear the embedder accessible on PBrowserBridge.
This ensures that the wrong accessible can't accidentally be used when an OOP iframe document is added while the iframe is hidden.
Differential Revision: https://phabricator.services.mozilla.com/D121015
Needed to tweak tests a bit because the code is now more descerning
wheter to fire an event or not. Will do so only if the state actually
changes.
Depends on D120901
Differential Revision: https://phabricator.services.mozilla.com/D120902
Also folded ARIAAttributeChanged into DOMAttributeChanged temporarily.
Will streamline that further in the next patch where state changes are
done a bit better.
Differential Revision: https://phabricator.services.mozilla.com/D120901
Allow for downstream projects such as Thunderbird to set different GUIDs for
AccessibleHandler to avoid clashes when both applications are installed.
The GUIDs themselves can be defined in confvars.sh or in branding/configure.sh
depending on the specific needs of the application. Fallback GUIDs are in
old-configure.
Differential Revision: https://phabricator.services.mozilla.com/D118124
Now that ProxyWrappers (RemoteAccessibleWrap and friends) are gone, IsProxy() can never be true, so checking it is pointless.
Differential Revision: https://phabricator.services.mozilla.com/D117536
Previously, when the cache was disabled, we had a RemoteAccessibleWrap for every RemoteAccessible.
This is no longer necessary and now only serves as an extra level of indirection and memory waste.
We still keep the stub MsaaAccessible to hold the id sent up from content.
Differential Revision: https://phabricator.services.mozilla.com/D117528
In a subsequent patch, we want to call it with a RemoteAccessible instead of a RemoteAccessibleWrap.
As an instance method of AccessibleWrap, this wouldn't be possible because AccessibleWrap is a subclass of LocalAccessible.
Strictly speaking, there's no good reason for this to be part of AccessibleWrap at all any more, but I'm not dealing with that here.
Differential Revision: https://phabricator.services.mozilla.com/D117527
This was previously used to wrap the COM proxy for the document child of an OOP iframe in a content process.
It was returned by OuterDocAccessible::LocalChildAt*.
Instead, BrowserBridgeChild now directly holds the COM proxy and MsaaAccessible traversal methods have specific code paths to return that as appropriate.
Since we no longer need the Windows OuterDocAccessible traversal overrides for OOP iframes nor remote top level documents, they have been removed entirely.
Differential Revision: https://phabricator.services.mozilla.com/D117526
It still can't handle being called on a RemoteAccessible because Bounds isn't unified yet.
However, this allows it to be called on a local OuterDoc (a XUL browser element) and return a remote document child.
Previously, this was handled by the RemoteAccessibleWrap returned from OuterDocAccessible::LocalChildAt, but that will go away in a subsequent patch.
Differential Revision: https://phabricator.services.mozilla.com/D117525
UnifiedComplete must stick around to serve as an mozIPlacesAutoComplete implementation for XUL consumers like search.js and privacy.js.
Differential Revision: https://phabricator.services.mozilla.com/D119306
This implements Jamie's suggested fixes for a screenreader issue when the
skeleton UI is enabled. Most of the work here is just pulling out pieces from the
files we needed to include in mozglue so that any references to, say, nsString
or other pieces from libxul either no longer exist or are only included when
building libxul. In a few cases this meant creating whole files to house single
functions, which isn't so pretty, but it was the best I could come up with to
get the job done.
Differential Revision: https://phabricator.services.mozilla.com/D117663
This implements Jamie's suggested fixes for a screenreader issue when the
skeleton UI is enabled. Most of the work here is just pulling out pieces from the
files we needed to include in mozglue so that any references to, say, nsString
or other pieces from libxul either no longer exist or are only included when
building libxul. In a few cases this meant creating whole files to house single
functions, which isn't so pretty, but it was the best I could come up with to
get the job done.
Differential Revision: https://phabricator.services.mozilla.com/D117663
We are going to disallow having any remote <browser>s or <iframe>s inside
nsDeckFrame, so this test is no longer necessary.
Differential Revision: https://phabricator.services.mozilla.com/D119066
This implements Jamie's suggested fixes for a screenreader issue when the
skeleton UI is enabled. Most of the work here is just pulling out pieces from the
files we needed to include in mozglue so that any references to, say, nsString
or other pieces from libxul either no longer exist or are only included when
building libxul. In a few cases this meant creating whole files to house single
functions, which isn't so pretty, but it was the best I could come up with to
get the job done.
Differential Revision: https://phabricator.services.mozilla.com/D117663
For OOP iframes, a doc load complete event on the embedder document doesn't necessarily mean the iframe document has finished loading yet.
We already checked for the busy state on the nested iframe doc and waited for a doc load complete event in that case.
However, depending on timing, it might be possible there is no document at all (even a busy one), so we should wait for doc load complete in that case too.
This is a speculative fix; I wasn't able to reproduce this myself.
Differential Revision: https://phabricator.services.mozilla.com/D118123
Returning E_NOINTERFACE for IUnknown at least is probably bad form.
Also, mscom::Interceptor seems to expect IDispatch to always be available and asserts if it isn't.
We're seeing some weird COM crashes in the wild, and while I don't know if this will fix them, correctness seems like a good place to start.
Differential Revision: https://phabricator.services.mozilla.com/D117967
This speeds up reflows on pages like view source that have a single
giant container. But this maybe slows down other cases? I'm not 100%
sure.
Differential Revision: https://phabricator.services.mozilla.com/D117948
This speeds up reflows on pages like view source that have a single
giant container. But this maybe slows down other cases? I'm not 100%
sure.
Differential Revision: https://phabricator.services.mozilla.com/D117948
An OuterDocAccessible can be recreated, causing the embedder accessible for a BrowserBridgeParent (OOP iframe) to change.
However, changing the src of an iframe can also cause a new BrowserBridgeParent to be created.
Previously, if addition of the child document was still pending when this occurred (because the OuterDocAccessible hadn't been sent to the parent yet), this pending addition could remain, causing problems if the id was reused later.
To fix this (and to hopefully make this more robust given the continued problems we're seeing in the wild with this area of the code), I've completely refactored the way we handle these pending child doc additions.
Rather than tracking the pending additions by their accessible id and child doc, we track them by their BrowserBridgeParent.
This way, we're closest to a single source of truth.
We also remove a pending addition when an associated BrowserBridgeParent is destroyed.
Differential Revision: https://phabricator.services.mozilla.com/D117889
An OuterDocAccessible can be recreated, causing the embedder accessible for a BrowserBridgeParent (OOP iframe) to change.
However, changing the src of an iframe can also cause a new BrowserBridgeParent to be created.
Previously, if addition of the child document was still pending when this occurred (because the OuterDocAccessible hadn't been sent to the parent yet), this pending addition could remain, causing problems if the id was reused later.
To fix this (and to hopefully make this more robust given the continued problems we're seeing in the wild with this area of the code), I've completely refactored the way we handle these pending child doc additions.
Rather than tracking the pending additions by their accessible id and child doc, we track them by their BrowserBridgeParent.
This way, we're closest to a single source of truth.
We also remove a pending addition when an associated BrowserBridgeParent is destroyed.
Differential Revision: https://phabricator.services.mozilla.com/D117889
ATK, Windows and XPCOM expect aria attribute keys to be stripped of
their aria- prefix. We should still store the item using the aria_ atom
and then strip the prefix when converting the key to a string.
Differential Revision: https://phabricator.services.mozilla.com/D116786
I'm not sure in what markup/style scenario a list item accessible
wouldn't have a frame, but would have a bullet. Apparently that can
happen in the wild. Added an assertion, hopefully we can trip it on
something reproducable in the future.
Differential Revision: https://phabricator.services.mozilla.com/D117347
This isn't used and thus isn't necessary.
Also, we can currently only support 127 content processes because of this id allocation, so avoiding this removes that restriction.
Differential Revision: https://phabricator.services.mozilla.com/D116206
Sending these doesn't really do any harm, as we already disregard them in the parent process.
However, it's certainly unnecessary and wasteful.
Differential Revision: https://phabricator.services.mozilla.com/D116204
1. GetHWNDFor and FireWinEvent now take an Accessible and support RemoteAccessible.
2. The Proxy*Event methods pass a RemoteAccessible instead of a RemoteAccessibleWrap, since RemoteAccessibleWraps aren't used with the cache enabled. FireWinEvent retrieves the RemoteAccessibleWrap if the cache is disabled.
Differential Revision: https://phabricator.services.mozilla.com/D116194
1. Make MsaaDocAccessible id maps contain Accessibles instead of AccessibleWraps. This allows us to put RemoteAccessibles in the id map.
2. Make MsaaAccessible::MsaaShutdown and GetChildIDFor handle RemoteAccessibles. GetChildIDFor lazily generates an id, Just as it does for LocalAccessible.
Differential Revision: https://phabricator.services.mozilla.com/D116193
1. Add support for RemoteAccessible in MsaaAccessible::GetFrom.
2. Add an overload of MsaaDocAccessible::GetFrom which accepts a (remote) DocAccessibleParent.
3. Add MsaaDocAccessible::GetFromOwned to return the containing document's MsaaDocAccessible for a given Accessible.
4. MsaaAccessible::LocalAcc now returns null for a RemoteAccessible, rather than just assuming the accessible is local.
5. MsaaAccessible::NativeAccessible now supports retrieving a native client pointer for LocalAccessibles, RemoteAccessibles and RemoteAccessibleWraps.
Differential Revision: https://phabricator.services.mozilla.com/D116192
With the cache disabled (which was previously always the case), we create a RemoteAccessibleWrap for each RemoteAccessible, stored as the RemoteAccessible's "Wrapper".
This in turn creates an MsaaAccessible.
However, both of these are little more than stubs: the MsaaAccessible itself is never returned to clients and most of the methods would crash if called.
They exist only to store the MSAA id received from the content process and to return the COM proxy from the content process to clients.
With the cache enabled, we now create a real MsaaAccessible for each RemoteAccessible, stored as the RemoteAccessible's "Wrapper".
This MsaaAccessible is directly returned to clients.
Soon, it will generate its own id in the parent process and will delegate to the underlying RemoteAccessible to serve queries from clients.
As part of this:
1. We stop managing COM proxies in the parent process when the cache is enabled, since we don't need those and can't store them anyway.
2. We stop setting the id on the MsaaAccessible when the cache is enabled, since it will soon generate its own id like local MsaaAccessibles do.
3. OuterDocAccessible::ChildCount had to be tweaked so it doesn't try to return a RemoteAccessibleWrap when the cache is enabled. (It previously called RemoteChildDocAccessible, which fetches a RemoteAccessibleWrap.)
Differential Revision: https://phabricator.services.mozilla.com/D116191
This is a security product and has no good reason to instantiate a11y.
We've also seen it show up in obscure crash reports.
Differential Revision: https://phabricator.services.mozilla.com/D116536
James Teh
Jonathan Kew
Eitan Isaacson
Chris Peterson
Marian-Vasile Laza
Morgan Reschenberg
Olli Pettay
Butkovits Atila
Niklas Baumgardner
Csoregi Natalia
Edgar Chen
Andi-Bogdan Postelnicu
Michelle Goossens
Gijs Kruitbosch
Ryan VanderMeulen
Emilio Cobos Álvarez
Ben Hearsum
Sebastian Streich
Alexandru Michis
Iulian Moraru
Dorel Luca
Narcis Beleuzu
Mike Hommey
Noemi Erli
Christoph Kerschbaumer
Sandor Molnar
Rob Lemley
Kashav Madan
Florian Quèze
Tooru Fujisawa
Harry Twyford
Paul Bone
Doug Thayer
Hiroyuki Ikezoe
Kagami Sascha Rosylight
Mats Palmgren
Ed Lee
Masayuki Nakano
Andrew Halberstadt