This change gives a bit more flexibility for isolating issues and seeing which specific marker offset is giving trouble when a test fails.
Depends on D168445
Differential Revision: https://phabricator.services.mozilla.com/D168446
Otherwise, semantics are exposed (e.g. for a table), which completely defeats the author's intent that this be treated as presentational.
Differential Revision: https://phabricator.services.mozilla.com/D170165
Previously, the test was retrieving an Accessible with a non-existent id and calling testAccessibleTree on it.
That meant we were calling testAccessibleTree with null, which is a no-op.
Now, we get the intended Accessible so that the test actually tests what it was supposed to test.
This is important to test a potential bug in the next patch.
Differential Revision: https://phabricator.services.mozilla.com/D170306
Otherwise, semantics are exposed (e.g. for a table), which completely defeats the author's intent that this be treated as presentational.
Differential Revision: https://phabricator.services.mozilla.com/D170165
Previously, the test was retrieving an Accessible with a non-existent id and calling testAccessibleTree on it.
That meant we were calling testAccessibleTree with null, which is a no-op.
Now, we get the intended Accessible so that the test actually tests what it was supposed to test.
This is important to test a potential bug in the next patch.
Differential Revision: https://phabricator.services.mozilla.com/D170306
nsImageFrame has support for displaying style URIs / an owned image
request, so use it.
The main behavior difference is that we don't fire `load` / `error`
events for those images anymore, but I don't see any event listener for
those around, so I think they can go.
Differential Revision: https://phabricator.services.mozilla.com/D168958
Previously, we retrieved an ancestor frame and various rects and used a transform to make continuation char rects relative to their primary frame.
Since all we need is an offset, we can just use nsIFrame::GetOffsetTo, which does all of this for us.
Differential Revision: https://phabricator.services.mozilla.com/D169446
This uses a bitfield for flags in FindBoundary, the idea is that it
will give some regression coverage if/when we move the native API
to do the same.
Differential Revision: https://phabricator.services.mozilla.com/D169246
Mostly changing XUL attributes to CSS properties, though there are a few
tricky ones:
* test_offsets.xhtml expects the scroller to be full-width, while
modern flexbox would honor width: 200px (so just remove it).
* window_intrinsic_size.xhtml was relying on the div imposing a XUL
min-size (the test is for SetSizeConstraints, bug 1447056). Use
min-height instead as that's what modern flexbox reads. Confirmed
that bug doesn't regress in any case.
* object-fit-contain-png-001.xhtml has a float: left which had no
effect on -moz-box, but which assert with modern layout[1]. In the
future I think we could remove that assert but anyways, for now just
keeping behavior.
* image-size.xhtml has a couple uninvestigated sizing differences. They
didn't seem problematic.
* 579323-1-ref.html changes because the other file has a canvas with
width=100 height=100 which imposes an aspect ratio (which XUL never
honored).
* window_largemenu.xhtml shows a regression, but this never worked on
some platforms (at least Linux+Wayland) and nobody has noticed it in
the browser area, so I suspect we're fine.
[1]: https://searchfox.org/mozilla-central/rev/08362489086b10de96e7a199b267ea5504c01583/layout/generic/ReflowInput.cpp#2137
Differential Revision: https://phabricator.services.mozilla.com/D169084
We only use these notifications from layout to push cache updates or insert Accessibles that were skipped during the initial build.
If the DocAccessible doesn't exist yet, creating it is pointless since we can't do this update until the tree is built.
The correct data will be included in the initial tree and cache push anyway.
Aside from pointless refresh ticks, this really shouldn't make any difference, since we don't build the initial tree until layout is ready anyway.
However, the only remotely relevant thing I can think of that's changed in the a11y code lately that might have caused a spike in these test failures is that bounds notifications might get fired earlier/more often from layout, potentially causing earlier creation of DocAccessibles.
Any change to timing might cause a shift in intermittent failures, and since this is wasteful anyway, we may as well fix it.
Differential Revision: https://phabricator.services.mozilla.com/D168911
We need to know about position: fixed in the parent process a11y cache, so we always need an Accessible for such elements.
We don't cache the fact that something is position: sticky, but we need the Accessible so that it can be notified about bounds updates when it moves within its container.
Previously, we calculated bounds incorrectly if a position: fixed/sticky element wasn't included in the a11y tree.
Differential Revision: https://phabricator.services.mozilla.com/D168916
Mostly changing XUL attributes to CSS properties, though there are a few
tricky ones:
* test_offsets.xhtml expects the scroller to be full-width, while
modern flexbox would honor width: 200px (so just remove it).
* window_intrinsic_size.xhtml was relying on the div imposing a XUL
min-size (the test is for SetSizeConstraints, bug 1447056). Use
min-height instead as that's what modern flexbox reads. Confirmed
that bug doesn't regress in any case.
* object-fit-contain-png-001.xhtml has a float: left which had no
effect on -moz-box, but which assert with modern layout[1]. In the
future I think we could remove that assert but anyways, for now just
keeping behavior.
* image-size.xhtml has a couple uninvestigated sizing differences. They
didn't seem problematic.
* 579323-1-ref.html changes because the other file has a canvas with
width=100 height=100 which imposes an aspect ratio (which XUL never
honored).
* window_largemenu.xhtml shows a regression, but this never worked on
some platforms (at least Linux+Wayland) and nobody has noticed it in
the browser area, so I suspect we're fine.
[1]: https://searchfox.org/mozilla-central/rev/08362489086b10de96e7a199b267ea5504c01583/layout/generic/ReflowInput.cpp#2137
Differential Revision: https://phabricator.services.mozilla.com/D169084
This revision changes the logic of MustCreateAccessible such that we always create
an accessible if the content's frame has been transformed and it has children.
This ensures that we have accessibles to which we apply transforms when
calculating accessible bounds. This revision also adds tests to verify that the
accessible is created, even when the element has role="none", both initially and
as a result of an on-the-fly style change.
Differential Revision: https://phabricator.services.mozilla.com/D167760
Mostly changing XUL attributes to CSS properties, though there are a few
tricky ones:
* test_offsets.xhtml expects the scroller to be full-width, while
modern flexbox would honor width: 200px (so just remove it).
* window_intrinsic_size.xhtml was relying on the div imposing a XUL
min-size (the test is for SetSizeConstraints, bug 1447056). Use
min-height instead as that's what modern flexbox reads. Confirmed
that bug doesn't regress in any case.
* object-fit-contain-png-001.xhtml has a float: left which had no
effect on -moz-box, but which assert with modern layout[1]. In the
future I think we could remove that assert but anyways, for now just
keeping behavior.
* image-size.xhtml has a couple uninvestigated sizing differences. They
didn't seem problematic.
* 579323-1-ref.html changes because the other file has a canvas with
width=100 height=100 which imposes an aspect ratio (which XUL never
honored).
* window_largemenu.xhtml shows a regression, but this never worked on
some platforms (at least Linux+Wayland) and nobody has noticed it in
the browser area, so I suspect we're fine.
[1]: https://searchfox.org/mozilla-central/rev/08362489086b10de96e7a199b267ea5504c01583/layout/generic/ReflowInput.cpp#2137
Differential Revision: https://phabricator.services.mozilla.com/D169084
This ended up being a lot more straight-forward than the menu changes.
TLDR:
* nsMenuBarFrame -> XULMenuBarElement
* nsMenuBarListener -> MenuBarListener
Rest should be rather straight-forward.
Depends on D168649
Differential Revision: https://phabricator.services.mozilla.com/D167809
Calculating it relative to an ancestor causes problems when the frame ancestry and the a11y ancestry diverge.
For example, this can happen for a transform which is also position: absolute.
In that case, the parent Accessible's frame is not an ancestor of the child Accessible's frame.
Since the transform is no longer relative to the ancestor, we no longer need to remove our parent-relative bounds before applying it.
However, we do need to ensure that we apply the transform *before* adding parent-relative bounds.
Differential Revision: https://phabricator.services.mozilla.com/D169059
This ended up being a lot more straight-forward than the menu changes.
TLDR:
* nsMenuBarFrame -> XULMenuBarElement
* nsMenuBarListener -> MenuBarListener
Rest should be rather straight-forward.
Depends on D168649
Differential Revision: https://phabricator.services.mozilla.com/D167809
No implementation of nsIDOMXULMenuListElement returns an input field
(this is menulist.js and autocomplete-input.js).
It seems autocomplete-input used to do this, but it got refactored to
extend HTMLInputElement instead, so this code can all go.
Differential Revision: https://phabricator.services.mozilla.com/D169066
The accessibility service and our COM objects can only be used on the main thread.
COM rules should mean that we never get direct calls on a background thread, but apparently, someone isn't following the rules.
Differential Revision: https://phabricator.services.mozilla.com/D168914
RetrieveCachedBounds returns Nothing() if mCachedFields is null.
Since we already have a large block which is only executed if RetrieveCachedBounds returned something, just move the call to IsFixedPos inside that block.
Differential Revision: https://phabricator.services.mozilla.com/D168642
Most usage is a straight replacement but gtk needs extra changes as it transfers plain text in UTF8 natively and needs to be converted into UTF16, and Windows uses single-byte characters for RTF and CF_HTML formats so we preserve this.
Differential Revision: https://phabricator.services.mozilla.com/D158587
We call this function on every ancestor when calculating bounds.
RemoteParent() currently requires a hash lookup, so it's more efficient to early return for !IsDoc() first.
This is a micro-optimisation, but it might have some impact given that we call this on every ancestor, especially when hit testing, where we call Bounds() a lot.
As a bit of drive-by cleanup, use RemoteParent() rather than calling Parent() and IsRemote/AsRemote().
Differential Revision: https://phabricator.services.mozilla.com/D168346
The set of moved LocalAccessibles on a document should only include Accessibles within that document.
Previously, we were descending into iframe documents.
This resulted in notifying the parent process about incorrect ids being moved and potentially pushing the cache in an invalid state.
Differential Revision: https://phabricator.services.mozilla.com/D168310
When an Accessible is moved, it's possible it is re-parented.
In that case, since our cached bounds are relative to the parent, the bounds are now incorrect.
To fix this, queue a bounds cache update whenever an Accessible is moved.
This will also trigger when an Accessible remains under the same parent.
However, because we cache bounds in LocalAccessible, we won't actually push a cache update unless the bounds really changed.
Differential Revision: https://phabricator.services.mozilla.com/D167866
This revision changes the logic of MustCreateAccessible such that we always create
an accessible if the content's frame has been transformed. This ensures that we
have accessibles to which we apply transforms when calculating accessible bounds.
This revision also adds a test to verify that the accessible is created, even
when the element has role="presentation".
Differential Revision: https://phabricator.services.mozilla.com/D167760
If a container has a 0 width/height, we use the ink overflow rect.
A container can overflow beyond the top left of its main rect; e.g. with align-items: flex-end.
Previously, when calculating the parent relative bounds of children, we always calculated relative to the parent's main rect.
If we ended up using the ink overflow rect for the parent, this meant that the parent relative bounds of children were wrong.
To fix this, compensate for the ink overflow offset in this case.
Differential Revision: https://phabricator.services.mozilla.com/D167636
A frame doesn't have to be reflowed to change its position.
For example, if there is a container c followed by a node outside the container o, inserting a node into c reflows c, but moves o down the page without reflowing o.
In this case, we previously weren't being notified that there was a possible bounds change, which meant we weren't updating the cache.
Now, we get notified about frames moving regardless of reflow.
Since this notification includes changes to CSS left/right/top/bottom, we can also remove the code added in bug 1774705 to explicitly watch for changes to these properties.
Differential Revision: https://phabricator.services.mozilla.com/D167645
Move it to the mozilla::widget namespace.
Use enum classes for transparency, popup type, popup level, etc.
Mostly automated with sed, but there were a few manual changes required
as well in windows code because they relied on Atomic<TransparencyMode>
working (which now doesn't because TransparencyMode is 1 byte instead of
4 bytes).
Differential Revision: https://phabricator.services.mozilla.com/D167537
There is an issue that when the element is hidden because its ancestor has 'content-visibility:hidden', it still appears in the accessibility tree.
Depending on CSS Containment Spec (https://www.w3.org/TR/css-contain-2/#cv-a11y),
it should be omitted from the accessibility tree.
This change fixes the issue above.
The approach is not to create the node in accessibility tree if the element has any ancestor specified with 'content-visibility:hidden'.
This patch can be tested such as:
(1) Check if the accessibility tree is created correctly
./mach test accessible/tests/browser/tree/browser_css_content_visibility.js
(2) Check if the accessibility tree is updated correctly
./mach test accessible/tests/browser/e10s/browser_treeupdate_csscontentvisibility.js
Differential Revision: https://phabricator.services.mozilla.com/D159879
For deepest child, previously, if the requested point was inside a different subtree to the origin Accessible (e.g. out-of-flow), we would return null because the origin wasn't an ancestor of the target.
We still check the ancestry, but if the point is inside the origin, we return the origin.
This is consistent with LocalAccessible (and thus non-cached RemoteAccessible).
For direct child, previously, we would return a child of the origin even if another element in a different subtree was higher in the z-order.
This is because we kept walking the viewport cache until `this`.
Instead, we now stop at the deepest match and walk up to the direct child if the origin is an ancestor.
If it isn't an ancestor, we still fall back to the origin if the point is inside it, just as we do for deepest child.
This change is necessary here because our test for out-of-flow hit testing changes the CSS left property to move an out-of-flow element.
That test passed because the bounds cache for that element was never updated, so we always found the target we expected.
In the next patch, we fix the bounds cache update, which would break this test without this hit testing fix.
In addition, this patch also changes our test for overlapping elements to match this new expectation.
There were also some bugs in the test that had to be fixed; e.g. using y instead of height to get the centre.
Since this is now consistent, we can remove the cache restriction and test LocalAccessible as well.
Differential Revision: https://phabricator.services.mozilla.com/D166849
Since language is also exposed as a text attribute and we cache all text attributes, we already cache this.
Thus, the RemoteAccessibleBase implementation just fetches it from the text attributes cache.
Differential Revision: https://phabricator.services.mozilla.com/D166857
textareas contain an anonymous div which fires the scroll event.
We don't have an Accessible for this div, so previously, we were just ignoring the scroll event.
Now, we'll redirect it to the textarea and get its Accessible.
This ensures that we push a cache update so that the scroll position (and thus the bounds) are correct.
This also means that a11y scroll events are fired on textareas where they weren't previously.
Differential Revision: https://phabricator.services.mozilla.com/D166508
Transition the group position calculation - specifically the code in Update -
to Pivot such that we can handle finding siblings across intervening generic
container accessibles, such as SECTIONs. This fixes issues with finding the
values of "pos in set" and "set size" for descendant owned elements in compound
ARIA widgets. This revision also updates and adds tests for this functionality.
Differential Revision: https://phabricator.services.mozilla.com/D165756
These MathML table elements may not have a table display style, in which case
they shouldn't be HTML table accessibles. This revision changes the logic of
MathMLMarkupMap such that mtable, mtr elements create generic ARIA accessibles
if the display style for the element is something other than 'table'. This
revision also adds a test to verify that the roles are still reported correctly,
even with this change.
Differential Revision: https://phabricator.services.mozilla.com/D165980
When an element's slot changes and the element is unslotted, we remove its Accessible.
If the body is moved inside a shadow host at the same time, we will process the slot removal first because moves are processed async.
Previously, this caused us to try to remove the DocAccessible (since it was still associated with the body), causing nastiness and potentially crashes.
We should never try to remove the DocAccessible, so explicitly prevent this when handling slot changes.
An assertion was also added to make sure we never try to remove the DocAccessible in future.
Differential Revision: https://phabricator.services.mozilla.com/D166134
This test passes on automation, somehow, probably due to timing.
Locally the click rolls up the popup (so you'd need a second click to
show the other popup).
This happens both with and without the patch, and given I debugged and
it is trivial to fix, well, make sure to hide the previously open menu
before clicking on another...
Differential Revision: https://phabricator.services.mozilla.com/D165983
Move most the event handling stuff to the DOM. I've left nsMenuBarFrame
for now, but I will be removing that in the future.
The basic set up is:
* nsMenuParent becomes XULMenuParentElement (menubar or popup, manages
the current active menu item)
* nsMenuFrame -> XULButtonElements that return true for IsMenu().
Can't use XULMenuElement because of <button type=menu>, which
behaves like a, well, menu.
This makes the a11y events for menus (DOMMenuItem{Active,Inactive}) make
sense (before that we were firing duplicate Inactive events etc, and the
event order was rather suspicious).
Differential Revision: https://phabricator.services.mozilla.com/D164210
Move most the event handling stuff to the DOM. I've left nsMenuBarFrame
for now, but I will be removing that in the future.
The basic set up is:
* nsMenuParent becomes XULMenuParentElement (menubar or popup, manages
the current active menu item)
* nsMenuFrame -> XULButtonElements that return true for IsMenu().
Can't use XULMenuElement because of <button type=menu>, which
behaves like a, well, menu.
This makes the a11y events for menus (DOMMenuItem{Active,Inactive}) make
sense (before that we were firing duplicate Inactive events etc, and the
event order was rather suspicious).
Differential Revision: https://phabricator.services.mozilla.com/D164210
HTMLSelectEventListener changes are needed, since currently that code works somewhat by accident given that
mTime often contains totally bogus values, like PR_IntervalNow(). Those changes then reveal issues also in
browser_editAddressDialog.js.
Differential Revision: https://phabricator.services.mozilla.com/D165618
My patch in bug 1805414 makes this fail in a way that's perma, and it
seems it is because we're not waiting for the actual value change to
happen.
Differential Revision: https://phabricator.services.mozilla.com/D165238
If the slot attribute is changed to assign the element to a slot which doesn't exist in the shadow DOM, the element is now unslotted and thus shouldn't be rendered.
We have code in PruneOrInsertSubtree to handle changes like this, but that only works if the frame for the shadow host or something inside the shadow DOM gets reconstructed.
In the case of a shadow host with a single child, there might not necessarily be any frame reconstruction.
To handle this, we listen for changes to the slot attribute.
We check if the element is still part of the flat tree (i.e. it is slotted).
If it isn't, we remove its Accessible.
Differential Revision: https://phabricator.services.mozilla.com/D165135
Certain MathML elements such as annotation and annotation-xml don't normally get an Accessible.
However, we force create Accessibles in some cases; e.g. if the element is focusable.
When this happens for these MathML elements (e.g. annotation-xml with a tabindex), we previously created AccessibleWraps which don't support text.
This meant that text formatting information was unavailable and caused assertions when pushing the cache.
To fix this, use HyperTextAccessibleWrap instead.
As a drive-by fix, also use HyperTextAccessibleWrap instead of HyperTextAccessible for content MathML elements.
This was almost certainly a typo when this was implemented.
This wouldn't have been noticeable in tests and some native platforms, but some platforms (e.g. Mac and Windows) do have some overrides in HyperTextAccessibleWrap, so we should use those.
Differential Revision: https://phabricator.services.mozilla.com/D165252
When applying an ancestor transform, ApplyTransform is supplied with the cumulative bounds calculated so far; i.e. from descendants.
When removing the parent relative offset, the code previously set the top left to (0, 0).
This not only removed the parent relative offset, but also the cumulative offset calculated for all descendants!
Instead, we now pass in the cumulative bounds as well as the parent relative bounds.
We only subtract the parent relative bounds.
Differential Revision: https://phabricator.services.mozilla.com/D164995
When applying an ancestor transform, ApplyTransform is supplied with the cumulative bounds calculated so far; i.e. from descendants.
When removing the parent relative offset, the code previously set the top left to (0, 0).
This not only removed the parent relative offset, but also the cumulative offset calculated for all descendants!
Instead, we now pass in the cumulative bounds as well as the parent relative bounds.
We only subtract the parent relative bounds.
Differential Revision: https://phabricator.services.mozilla.com/D164995
Previously, this test case caused an assertion, but that was fixed in bug 1792120.
We also ensure the x and y are the same before an dafter the re-creation.
This verifies that we aren't losing the iframe's border/padding.
Differential Revision: https://phabricator.services.mozilla.com/D164879
Aside from making the code consistent for OOP and in-process iframes, this also fixes incorrect bounds in in-process iframe documents when the iframe's padding is changed.
We always pushed a cache update for the iframe when the padding was changed.
However, we previously relied on parent-relative bounds for in-process iframes, which didn't get updated when the iframe's padding changed.
Differential Revision: https://phabricator.services.mozilla.com/D164878
We previously thought that DocAccessible::GetFrame didn't return a frame.
On the contrary, it has returned the PresShell's root frame for a long time now, which is precisely what we want.
This means we can make this method a bit cleaner.
Differential Revision: https://phabricator.services.mozilla.com/D164877
See the code comments for an explanation.
This fixes assertions and crashes when pushing the cache when a page contains a map which is unslotted in a shadow host.
Differential Revision: https://phabricator.services.mozilla.com/D164783
Calling GetAccessibleOrContainer meant that if a particular content node didn't have an Accessible but did have a frame, we'd insert its container in its place.
There might have been other descendants of that container that did have Accessibles, though.
In that case, we would have put the container ahead of some of its descendants!
To fix this, use GetAccessible instead.
We'll still fall back to the container if the user hit tests a point in an inaccessible node, since it will appear later in the viewport cache.
Differential Revision: https://phabricator.services.mozilla.com/D164803
That patch was always somewhat hacky, but we couldn't figure out the cause of the problem.
Now that I've figured it out, a more correct solution is forthcoming.
Differential Revision: https://phabricator.services.mozilla.com/D164802
As of the prior patch, these are no longer needed. I removed
these with a script, then ran clang-format on the files, then
manually reverted a few unrelated changed from the formatter.
Differential Revision: https://phabricator.services.mozilla.com/D164829
These can contain <foreignobject>, which contains HTML but does not normally create its own Accessible.
This means that these Accessibles could have TextLeafAccessible children.
TextLeafAccessible children must always have a HyperTextAccessible as a parent.
Therefore, we must use HyperTextAccessible for <svg> and <g>.
As well as fixing assertions, this allows text formatting to be queried for <foreignobject> content, which was previously broken.
Differential Revision: https://phabricator.services.mozilla.com/D164636
Done:
- Functionality of the button was changed from cleaning the field value to toggling the datepicker dialog.
- Pre-existing issues resolved: Updated datetimebox.js to use `keydown` event instead of the deprecated `keypress` (which does not preventDefault for buttons), added default handling of digits for `keydown`, and added a check to avoid running duplicate cleanup when the picker is closed
- Removed ability to open a date picker from editable elements of the `<input type="date">` field and ensured keyboard and mouse/touch click are working for the Calendar button, while Escape functionality remained
- Updated `onBlur` logic for the button in accordance with its new functionaility
- New Calendar SVG icon was created by Katie Caldwell and optimized by Sam Foster
- Provided HCM support for the Calendar button
- Ensured the Calendar button is not shown on `<input type=time>` to preserve the existent UX
- Added Fluent l10n to the content process and provided `title` to the image button (SVG is marked as `role="none"` to avoid exposure to assistive technology)
- Added functional and markup tests for the Calendar button and its localization, updated Reset button tests to the Calendar one
ToDo (further patch):
1. Pt.4 - Ensure keyboard support when focus moves between processes
ToDo (other dependencies/bugs):
1. Investigations into if we should show a calendar button for read-only fields and if a Reset button would be benefitial to be shown for a `type=time` inputs
Depends on D139981
Differential Revision: https://phabricator.services.mozilla.com/D141175
This makes a couple of large changes:
(1) "Generic" buttons (the ones added by `UrlbarView.#addRowButton()`) are now
supported in all row types. The help button that's currently included in some
types of rows when `result.payload.helpUrl` is defined is now supported for all
row types, and two additional button types are now supported too: block buttons
and labeled buttons. A row will get a block button if its
`result.payload.isBlockable` is defined. It will get a labeled button if
`result.payload.buttons` is defined and non-empty. A button can include a `url`
property that is then added as an attribute on the button's element, and
`UrlbarInput.pickResult()` will use this attribute to load the URL when the
button is picked.
(2) The reason I added labeled buttons is because it lets us support tip buttons
without much more effort, which then lets us get rid of the special row type
used for tips. With this patch, tips are now standard rows that use generic
buttons.
This approach should be compatible with the result menu, when we switch over to
it, because we can include the help and block commands in the menu when
`helpUrl` and `isBlockable` are defined, instead of creating buttons for them.
Labeled buttons -- the ones used in tips -- would still be created. The result
menu button itself can continue to be a generic button.
It should also be compatible with including the result menu button inside the
row selection. We'll still add buttons to `.urlbarView-row`, separate from
`.urlbarView-row-inner`, so that the buttons can continue to be on the right
side of the row. We can color the background of the row instead of the
row-inner.
As with D163630, my motivation for this change is to support generic buttons in
dynamic result rows so that help and block buttons can be easily added to
weather suggestions. Here too the larger changes of supporting generic labeled
buttons and removing special rows for tips aren't strictly necessary, but I took
the opportunity to rework things.
Finally, this makes a few other changes:
* It includes some of the more minor improvements to selection that I made in
D163630.
* It removes the help URL code from quick actions since it was decided not to
show a help button. Currently, the button is hidden in CSS, but now that a
generic help button is added for dynamic result rows when
`result.payload.helpUrl` is defined, `helpUrl` needs to be removed from the
payload to prevent a button from being added.
* I removed the special tip wrapping behavior, where the tip button and help
button would wrap below the tip's text. Instead, now the text wraps inside
row-inner and the buttons always remain on the same horizontal as the text. I
don't think it's worth the extra complication.
Differential Revision: https://phabricator.services.mozilla.com/D163766
This makes a couple of large changes:
(1) "Generic" buttons (the ones added by `UrlbarView.#addRowButton()`) are now
supported in all row types. The help button that's currently included in some
types of rows when `result.payload.helpUrl` is defined is now supported for all
row types, and two additional button types are now supported too: block buttons
and labeled buttons. A row will get a block button if its
`result.payload.isBlockable` is defined. It will get a labeled button if
`result.payload.buttons` is defined and non-empty. A button can include a `url`
property that is then added as an attribute on the button's element, and
`UrlbarInput.pickResult()` will use this attribute to load the URL when the
button is picked.
(2) The reason I added labeled buttons is because it lets us support tip buttons
without much more effort, which then lets us get rid of the special row type
used for tips. With this patch, tips are now standard rows that use generic
buttons.
This approach should be compatible with the result menu, when we switch over to
it, because we can include the help and block commands in the menu when
`helpUrl` and `isBlockable` are defined, instead of creating buttons for them.
Labeled buttons -- the ones used in tips -- would still be created. The result
menu button itself can continue to be a generic button.
It should also be compatible with including the result menu button inside the
row selection. We'll still add buttons to `.urlbarView-row`, separate from
`.urlbarView-row-inner`, so that the buttons can continue to be on the right
side of the row. We can color the background of the row instead of the
row-inner.
As with D163630, my motivation for this change is to support generic buttons in
dynamic result rows so that help and block buttons can be easily added to
weather suggestions. Here too the larger changes of supporting generic labeled
buttons and removing special rows for tips aren't strictly necessary, but I took
the opportunity to rework things.
Finally, this makes a few other changes:
* It includes some of the more minor improvements to selection that I made in
D163630.
* It removes the help URL code from quick actions since it was decided not to
show a help button. Currently, the button is hidden in CSS, but now that a
generic help button is added for dynamic result rows when
`result.payload.helpUrl` is defined, `helpUrl` needs to be removed from the
payload to prevent a button from being added.
* I removed the special tip wrapping behavior, where the tip button and help
button would wrap below the tip's text. Instead, now the text wraps inside
row-inner and the buttons always remain on the same horizontal as the text. I
don't think it's worth the extra complication.
Differential Revision: https://phabricator.services.mozilla.com/D163766
This revision changes the logic in MathMLMarkupMap such that mtd elements create
generic ARIA grid accessibles if the display style for the element is something
other than 'table'. This revision also adds a test that verifies that the roles
remain as expected, even with this change.
Differential Revision: https://phabricator.services.mozilla.com/D163898
Done:
- Functionality of the button was changed from cleaning the field value to toggling the datepicker dialog.
- Pre-existing issues resolved: Updated datetimebox.js to use `keydown` event instead of the deprecated `keypress` (which does not preventDefault for buttons), added default handling of digits for `keydown`, and added a check to avoid running duplicate cleanup when the picker is closed
- Removed ability to open a date picker from editable elements of the `<input type="date">` field and ensured keyboard and mouse/touch click are working for the Calendar button, while Escape functionality remained
- Updated `onBlur` logic for the button in accordance with its new functionaility
- New Calendar SVG icon was created by Katie Caldwell and optimized by Sam Foster
- Provided HCM support for the Calendar button
- Ensured the Calendar button is not shown on `<input type=time>` to preserve the existent UX
- Added Fluent l10n to the content process and provided `title` to the image button (SVG is marked as `role="none"` to avoid exposure to assistive technology)
- Added functional and markup tests for the Calendar button and its localization, updated Reset button tests to the Calendar one
ToDo (further patch):
1. Pt.4 - Ensure keyboard support when focus moves between processes
ToDo (other dependencies/bugs):
1. Investigations into if we should show a calendar button for read-only fields and if a Reset button would be benefitial to be shown for a `type=time` inputs
Depends on D139981
Differential Revision: https://phabricator.services.mozilla.com/D141175
In bug 1766794, I replaced partial invalidation of cached offsets with full cache invalidation.
I did this to simplify the code, since the offsets cache was being extended to RemoteAccessible.
This is fine for RemoteAccessible because offsets are only needed there when clients query them.
However, for local HyperTextAccessible, offsets are always required to fire text events.
Full cache invalidation there meant that many insertions into a large container was very expensive.
To fix this, reinstate the ability to partially invalidate the offsets cache.
This is largely the code I previously removed from HyperTextAccessible, with some tweaks for readability.
Differential Revision: https://phabricator.services.mozilla.com/D163418
We will soon have an offsets array in the parent process cache which is not built in one shot.
Instead, we will gradually cache more offsets as they are requested.
To do this, we need a way to get an array from the cache which can be modified in-place.
Differential Revision: https://phabricator.services.mozilla.com/D163417
Done:
- Functionality of the button was changed from cleaning the field value to toggling the datepicker dialog.
- Pre-existing issues resolved: Updated datetimebox.js to use `keydown` event instead of the deprecated `keypress` (which does not preventDefault for buttons), added default handling of digits for `keydown`, and added a check to avoid running duplicate cleanup when the picker is closed
- Removed ability to open a date picker from editable elements of the `<input type="date">` field and ensured keyboard and mouse/touch click are working for the Calendar button, while Escape functionality remained
- Updated `onBlur` logic for the button in accordance with its new functionaility
- New Calendar SVG icon was created by Katie Caldwell and optimized by Sam Foster
- Provided HCM support for the Calendar button
- Ensured the Calendar button is not shown on `<input type=time>` to preserve the existent UX
- Added Fluent l10n to the content process and provided `title` to the image button (SVG is marked as `role="none"` to avoid exposure to assistive technology)
- Added functional and markup tests for the Calendar button and its localization, updated Reset button tests to the Calendar one
ToDo (further patch):
1. Pt.4 - Ensure keyboard support when focus moves between processes
ToDo (other dependencies/bugs):
1. Investigations into if we should show a calendar button for read-only fields and if a Reset button would be benefitial to be shown for a `type=time` inputs
Depends on D139981
Differential Revision: https://phabricator.services.mozilla.com/D141175
This revision addresses an issue with th elements within tables that don't have
the table CSS display style. In these cases, where we create an
ARIAGridCellAccessible for a th, we fail to report rowheader and columnheader
properly, since ARIAGridCellAccessible doesn't know how to handle th elements.
This revision fixes the problem by moving the th NativeRole logic into the
TableCellAccessible class, then calling that logic from both the ARIA grid cell
accessible NativeRole and from HTMLTableHeaderCellAccessible, as before. This
revision also updates tests reliant on the old behavior, including beefing up
an existing test aimed at this bug specifically.
Differential Revision: https://phabricator.services.mozilla.com/D163371
Done:
- Functionality of the button was changed from cleaning the field value to toggling the datepicker dialog.
- Pre-existing issues resolved: Updated datetimebox.js to use `keydown` event instead of the deprecated `keypress` (which does not preventDefault for buttons), added default handling of digits for `keydown`, and added a check to avoid running duplicate cleanup when the picker is closed
- Removed ability to open a date picker from editable elements of the `<input type="date">` field and ensured keyboard and mouse/touch click are working for the Calendar button, while Escape functionality remained
- Updated `onBlur` logic for the button in accordance with its new functionaility
- New Calendar SVG icon was created by Katie Caldwell and optimized by Sam Foster
- Provided HCM support for the Calendar button
- Ensured the Calendar button is not shown on `<input type=time>` to preserve the existent UX
- Added Fluent l10n to the content process and provided `title` to the image button (SVG is marked as `role="none"` to avoid exposure to assistive technology)
- Added functional and markup tests for the Calendar button and its localization, updated Reset button tests to the Calendar one
ToDo (further patch):
1. Pt.4 - Ensure keyboard support when focus moves between processes
ToDo (other dependencies/bugs):
1. Investigations into if we should show a calendar button for read-only fields and if a Reset button would be benefitial to be shown for a `type=time` inputs
Depends on D139981
Differential Revision: https://phabricator.services.mozilla.com/D141175
The pivot should never cross from remote into parent process local
containers. We need to explicitly set the root for the pivot, and
assert for this in the pivot traversal methods.
Differential Revision: https://phabricator.services.mozilla.com/D163285
This can happen if we're pushing the tree for only moved (no new) Accessibles, since we don't send cache pushes for those.
It can also happen if an Accessible generates no cache data, which is rare but probably not impossible.
Making IPDL calls with no data is wasteful, so don't do it.
Differential Revision: https://phabricator.services.mozilla.com/D163203
Trying to access a local OuterDocAccessible from the Android UI thread was causing a crash.
We shouldn't be crossing document boundaries anyway.
Differential Revision: https://phabricator.services.mozilla.com/D163067
After bug 1779156, show events were fired from RecvCache, rather than from RecvShowEvent.
This was done to ensure that cache data was available when the event was fired.
However, because RecvCache fired a show event for every initial cache push, this meant that it also fired one for the document itself, plus all the document's initial direct children.
Firing an event for the document caused problems for ATK, since the parent was null for all top level documents.
This also meant we were firing a lot of unnecessary show events, which could be a performance problem for documents with a lot of initial direct children.
To fix this, provide an explicit argument to PDocAccessible::Cache specifying whether to dispatch a show event or not.
This replaces the existing aFinal argument, which was never used.
Differential Revision: https://phabricator.services.mozilla.com/D163192
This never really belonged in XULMenuItemAccessibleWrap::Name, as it's specific to MSAA.
This will allow us to remove this Wrap class.
Differential Revision: https://phabricator.services.mozilla.com/D162455
When a selection event is dropped due to coalescence, we still include the impacted Accessible in the SelectedAccessiblesChanged notification we send to the parent process.
Although we skip events with defunct targets, we weren't skipping defunct items referenced by selection events.
This meant that if an Accessible was selected/unselected but was shut down before we sent SelectedAccessiblesChanged, the notification would include a dead Accessible.
This was causing an assertion in the parent process.
To fix this, we now ignore defunct items in selection events.
Differential Revision: https://phabricator.services.mozilla.com/D162551
Gecko switched to mutating the tree structure for aria-owns many years ago instead of using relations, so this is now dead code.
Note that a test case was added for this aria-owns case in accessible/tests/mochitest/role/test_general.html in bug 1044431.
This still passes with this code removed.
Differential Revision: https://phabricator.services.mozilla.com/D162459
PruneOrInsertSubtree already has code to deal with this.
However, layout previously didn't notify a11y about the change if a shadow root was attached without any content, so PruneOrInsertSubtree was never called.
We now schedule re-evaluation of the a11y tree of the shadow host when DOM notifies layout about the attached shadow root.
This will call PruneOrInsertSubtree during the next refresh tick.
Differential Revision: https://phabricator.services.mozilla.com/D162263
When we push a cache update for tables, we call IsProbablyLayoutTable.
That in turn checks whether the first grandchild LocalAccessible of each row is an abbreviation.
If there is a malformed table containing an iframe as a child of a row, this grandchild will be an embedded DocAccessible.
Since a DocAccessible has a null mContent prior to DoInitialUpdate, calling IsAbbreviation on this would previously crash because it didn't null check mContent.
The fix is simply to null check mContent.
Differential Revision: https://phabricator.services.mozilla.com/D162360
This reduces memory used by the cache, since the same font family and language strings often occur many times in the same document (and likely even across documents).
Differential Revision: https://phabricator.services.mozilla.com/D162276
Layout doesn't consider these types of th elements to be table cells since there
is no underlying table layout. This patch asks layout whether it considers the
frame's accessible type to be a table cell accessible. If it does, continue to
use the HTMLTableHeaderCellAccessible, but - otherwise - use a generic grid cell
accessible. This revision also adds a test to verify the behavior of th in this
situation. This essentially repeats the logic already in existence for td.
Differential Revision: https://phabricator.services.mozilla.com/D162357
This doesn't change behavior, but clarifies a bit the naming to match
the scrollIntoView() API. Also makes the name generic (rather than
Top/Left/etc), since for scrollIntoView we want to make the axes be
logical.
That will be done in bug 1789464 (probably via an extra ScrollFlag).
Differential Revision: https://phabricator.services.mozilla.com/D162286
The code which calculated the focused state in RemoteAccessibleBase was incorrect.
It used the focused BrowserParent rather than the focused BrowsingContext to determine if the document was focused, which meant it treated a top level document as focused even when an embedded in-process iframe had focus.
FocusManager::FocusedAccessible (and now FocusManager::IsFocused) do the right thing, so we can use IsFocused instead.
LocalAccessible already used IsFocused to calculate the focused state.
Since both local and remote both now do the same thing, we can move this into Accessible::ApplyImplicitState.
Differential Revision: https://phabricator.services.mozilla.com/D161876
Previously, IsFocused had a separate implementation instead of just using FocusedAccessible because of cases years ago where it would unintentionally cause us to create a DocAccessible for an initial about:blank document.
As far as I can tell, we no longer prevent creation of DocAccessibles for initial documents because they aren't necessarily temporary documents.
Therefore, we can just compare against FocusedAccessible now.
Since FocusedAccessible is already unified, this makes unifying IsFocused very simple.
This paves the way for unifying correct calculation of the focused state.
This necessitated a change to FocusedLocalAccessible to prevent it from trying to get/create a DocAccessible when the accessibility service is shutting down.
Shutting down the service shuts down all documents, and shutting down a document now calls IsFocused, which causes us to get/create a DocAccessible.
That isn't safe while the service is shutting down and was causing a crash.
Differential Revision: https://phabricator.services.mozilla.com/D161875
IN subsequent patches, IsFocused will be called when querying the state of a RemoteAccessible and IsFocused calls FocusedAccessible.
A RemoteAccessible can be queried from the Android UI thread (which is different to Gecko's main thread), but it's not safe to deal with LocalAccessibles off the main thread.
Therefore, don't try to call FocusedLocalAccessible in this case.
Differential Revision: https://phabricator.services.mozilla.com/D162177
See the code comment for further details.
In subsequent patches, IsFocused is refactored to call FocusedAccessible, which asserts that mActiveItem is not defunct.
Without this active item fix, that change would cause accessible/tests/mochitest/actions/test_keys.xhtml to assert.
Differential Revision: https://phabricator.services.mozilla.com/D161887
This revision expands the reporting of ARIA combobox values to non-editable
comboboxes with no selected option. In these cases, according to the spec, we
should report a value "represented by its descendant elements [...] determined
using the same method used to compute the name of a button from its descendant
content." In this revision, I interpret that to mean, roughly, nsTextEquivUtils'
GetTextEquivFromSubtree, which is tantamount to the name calculation, except
that it crucially avoids rejecting the combobox for having eNameFromValueRule,
unlike GetNameFromSubtree. This revision updates this logic in LocalAccessible
and RemoteAccessibleBase, and adds tests to verify the above.
Differential Revision: https://phabricator.services.mozilla.com/D162229
The code which calculated the focused state in RemoteAccessibleBase was incorrect.
It used the focused BrowserParent rather than the focused BrowsingContext to determine if the document was focused, which meant it treated a top level document as focused even when an embedded in-process iframe had focus.
FocusManager::FocusedAccessible (and now FocusManager::IsFocused) do the right thing, so we can use IsFocused instead.
LocalAccessible already used IsFocused to calculate the focused state.
Since both local and remote both now do the same thing, we can move this into Accessible::ApplyImplicitState.
Differential Revision: https://phabricator.services.mozilla.com/D161876
Previously, IsFocused had a separate implementation instead of just using FocusedAccessible because of cases years ago where it would unintentionally cause us to create a DocAccessible for an initial about:blank document.
As far as I can tell, we no longer prevent creation of DocAccessibles for initial documents because they aren't necessarily temporary documents.
Therefore, we can just compare against FocusedAccessible now.
Since FocusedAccessible is already unified, this makes unifying IsFocused very simple.
This paves the way for unifying correct calculation of the focused state.
Differential Revision: https://phabricator.services.mozilla.com/D161875
IN subsequent patches, IsFocused will be called when querying the state of a RemoteAccessible and IsFocused calls FocusedAccessible.
A RemoteAccessible can be queried from the Android UI thread (which is different to Gecko's main thread), but it's not safe to deal with LocalAccessibles off the main thread.
Therefore, don't try to call FocusedLocalAccessible in this case.
Differential Revision: https://phabricator.services.mozilla.com/D162177
See the code comment for further details.
In subsequent patches, IsFocused is refactored to call FocusedAccessible, which asserts that mActiveItem is not defunct.
Without this active item fix, that change would cause accessible/tests/mochitest/actions/test_keys.xhtml to assert.
Differential Revision: https://phabricator.services.mozilla.com/D161887
The button tweak is needed because now if you have:
<button>
<label value="foo">
</button>
There is a text node for the value (generated content), and the <button>
shouldn't steal it.
I was getting crashes without it because XULButtonAccessible isn't
hypertext (so I wonder if the IsText() code-path can even be reached?).
Same issue with XULComboboxAccessible, fixed the same way by preventing
text there (preserving behavior).
Setting crop="center" now reframes, so test_label needs to change.
Differential Revision: https://phabricator.services.mozilla.com/D162011
This relation isn't part of the ATK and IA2 specs.
In ATK and the older way of retrieving relations in IA2, we have to calculate all relations.
Since LINKS_TO can be slow and it isn't supported by ATK and IA2 anyway, let's just not expose it at all for them.
Differential Revision: https://phabricator.services.mozilla.com/D161775
The code which calculated the focused state in RemoteAccessibleBase was incorrect.
It used the focused BrowserParent rather than the focused BrowsingContext to determine if the document was focused, which meant it treated a top level document as focused even when an embedded in-process iframe had focus.
FocusManager::FocusedAccessible (and now FocusManager::IsFocused) do the right thing, so we can use IsFocused instead.
LocalAccessible already used IsFocused to calculate the focused state.
Since both local and remote both now do the same thing, we can move this into Accessible::ApplyImplicitState.
Differential Revision: https://phabricator.services.mozilla.com/D161876
Previously, IsFocused had a separate implementation instead of just using FocusedAccessible because of cases years ago where it would unintentionally cause us to create a DocAccessible for an initial about:blank document.
As far as I can tell, we no longer prevent creation of DocAccessibles for initial documents because they aren't necessarily temporary documents.
Therefore, we can just compare against FocusedAccessible now.
Since FocusedAccessible is already unified, this makes unifying IsFocused very simple.
This paves the way for unifying correct calculation of the focused state.
Differential Revision: https://phabricator.services.mozilla.com/D161875
See the code comment for further details.
In subsequent patches, IsFocused is refactored to call FocusedAccessible, which asserts that mActiveItem is not defunct.
Without this active item fix, that change would cause accessible/tests/mochitest/actions/test_keys.xhtml to assert.
Differential Revision: https://phabricator.services.mozilla.com/D161887
It already has a mochitest and is probably old enough that we don't need
it. Loading XUL in content with e10s enabled is not something our cache
is prepared for yet.
Differential Revision: https://phabricator.services.mozilla.com/D162057
Otherwise, we crash when inspecting RemoteAccessible objects in Dev Tools if the cache is disabled or hasn't been received yet.
Differential Revision: https://phabricator.services.mozilla.com/D161872
On Android, we access RemoteAccessibles from the UI thread.
It's not safe to access LocalAccessibles on the UI thread.
The a11y code avoids touching LocalAccessibles on the UI thread directly.
However, Accessible::GetLevel was previously walking ancestry, which might hit a LocalAccessible and result in a crash.
To avoid this, don't ascend past a DocAccessible.
Level calculation shouldn't be crossing document boundaries anyway.
Note that we don't have to do this when we simply retrieve the parent (but nothing further) because we only do that for certain roles and the parent of these roles could never be an OuterDoc.
Differential Revision: https://phabricator.services.mozilla.com/D161873
This relation isn't part of the ATK and IA2 specs.
In ATK and the older way of retrieving relations in IA2, we have to calculate all relations.
Since LINKS_TO can be slow and it isn't supported by ATK and IA2 anyway, let's just not expose it at all for them.
Differential Revision: https://phabricator.services.mozilla.com/D161775
Eitan Isaacson
James Teh
Butkovits Atila
Emilio Cobos Álvarez
Florian Quèze
Razvan Cojocaru
Robert Longson
Cosmin Sabou
Iulian Moraru
Joel Maher
Gijs Kruitbosch
Andi-Bogdan Postelnicu
Cristina Horotan
Marian-Vasile Laza
Nathan LaPre
Morgan Rae Reschenberg
Neil Deakin
Mark Banner
Otto Länd
Sandor Molnar
Dão Gottwald
Jihye Hong
Ben Freist
Csoregi Natalia
Olli Pettay
Sylvestre Ledru
Marco Castelluccio
Stanca Serban
Andrew McCreight
Norisz Fay
lyavor
Anna Yeddi
Drew Willcoxon
Noemi Erli
Cristian Tuns
Michael Comella
Paul Bone
Ting-Yu Lin