Stop supporting following the system preference, but keep macOS users
able to switch to just text controls (accessibility.tabfocus=1) in the
settings.
Change the meaning of the "Use the tab key to move focus between form
controls and links" checkbox in the Firefox settings, which was
introduced in bug 1628476 to override the system setting.
The intention, I think was that this checkbox being off resulted in
"follow the system" behavior, but that didn't quite happen due to a bug
in the preferences code (this[1] won't unset the pref, because of
this[2], which means we'll just return 0).
This patch changes it so that the checkbox instead always ignores the
system setting. There will no longer be a Firefox setting (neither in
the UI nor on about:config) that means "follow system setting".
This allows us to somewhat simplify the approach compared to the
previous patch in D196110, and keep the accessibility.tabfocus working
as the source of truth without a migration.
In the future, we can think of migrating accessibility.tabfocus to a
boolean pref, which would allow us to do the cleanups to the preferences
code that D196110 did.
[1]: https://searchfox.org/mozilla-central/rev/f1532761de0b60337e42c6c3f525288a523dabef/browser/components/preferences/main.js#2252
[2]: https://searchfox.org/mozilla-central/rev/f1532761de0b60337e42c6c3f525288a523dabef/toolkit/content/preferencesBindings.js#450,483
Differential Revision: https://phabricator.services.mozilla.com/D208602
Stop supporting following the system preference, but keep macOS users
able to switch to just text controls (accessibility.tabfocus=1) in the
settings.
Change the meaning of the "Use the tab key to move focus between form
controls and links" checkbox in the Firefox settings, which was
introduced in bug 1628476 to override the system setting.
The intention, I think was that this checkbox being off resulted in
"follow the system" behavior, but that didn't quite happen due to a bug
in the preferences code (this[1] won't unset the pref, because of
this[2], which means we'll just return 0).
This patch changes it so that the checkbox instead always ignores the
system setting. There will no longer be a Firefox setting (neither in
the UI nor on about:config) that means "follow system setting".
This allows us to somewhat simplify the approach compared to the
previous patch in D196110, and keep the accessibility.tabfocus working
as the source of truth without a migration.
In the future, we can think of migrating accessibility.tabfocus to a
boolean pref, which would allow us to do the cleanups to the preferences
code that D196110 did.
[1]: https://searchfox.org/mozilla-central/rev/f1532761de0b60337e42c6c3f525288a523dabef/browser/components/preferences/main.js#2252
[2]: https://searchfox.org/mozilla-central/rev/f1532761de0b60337e42c6c3f525288a523dabef/toolkit/content/preferencesBindings.js#450,483
Differential Revision: https://phabricator.services.mozilla.com/D208602
KEY_F6 has a special requirement for chrome document navigation
such that if the focus algorithm has reached to the end of the
top-level chrome document, it expects to focus first element of
this chrome document, because the root element of chrome document
is not focusable. This patch is to ensure this behaviour is still
working.
dom/tests/mochitest/chrome/test_focus_docnav.xhtml is used to test this.
Differential Revision: https://phabricator.services.mozilla.com/D200864
In the test case, `nsFocusManager::GetSelectionLocation` is called with
collapsed selection at end of the `<svg>` which ends with collapsible
white-spaces. Therefore, it reaches the white-space only text node and it
does not have a primary frame due to invisible.
Previously, creating `nsFrameIterator` failed and then the method returned
error, but after bug 779684, `nsFrameIterator` constructor wants non-nullptr
frame avoiding crash. Therefore, I added the `MOZ_ASSERT` there to get a
way to reproduce the case and now we got it.
I think that just removing `MOZ_ASSERT` and keeping returning error is not
correct. The text can be invisible with the other reasons and it does not
collapsed at end of invisible text, this does not return error. Therefore,
this patch makes just returning the text in the case without error.
Differential Revision: https://phabricator.services.mozilla.com/D198128
See the comment in the bug, this is not a new crash. The old factory method
did the null-check and returned. Therefore, before using `MOZ_TRY`, calling
a method of `nsFrameIterator` with `nullptr` caused a crash.
I tried to reproduce this bug with creating empty text nodes or invisible
text nodes, but I couldn't reproduce this. Therefore, this patch does not
contain crash tests.
Differential Revision: https://phabricator.services.mozilla.com/D197317
Nobody shares an instance of `nsFrameIterator`. Therefore, we can make it
non-refcountable. Additionally, `nsVisualIterator` is not required because
it overrides only 4 methods which are really simple. So, once we merge it
into `nsFrameIterator`, we can allocate it in the stack.
Differential Revision: https://phabricator.services.mozilla.com/D197144
The class is used only for creating `nsFrameIterator`, but it's unnecessary
so that we can get rid of it and its interface.
Differential Revision: https://phabricator.services.mozilla.com/D197143
It's inherited only by `nsFrameIterator`, and `nsFrameIterator` can be declared
in a header file. So, the interface is not required and removing it can avoid
virtual calls.
Differential Revision: https://phabricator.services.mozilla.com/D197142
Nobody shares an instance of `nsFrameIterator`. Therefore, we can make it
non-refcountable. Additionally, `nsVisualIterator` is not required because
it overrides only 4 methods which are really simple. So, once we merge it
into `nsFrameIterator`, we can allocate it in the stack.
Differential Revision: https://phabricator.services.mozilla.com/D197144
The class is used only for creating `nsFrameIterator`, but it's unnecessary
so that we can get rid of it and its interface.
Differential Revision: https://phabricator.services.mozilla.com/D197143
It's inherited only by `nsFrameIterator`, and `nsFrameIterator` can be declared
in a header file. So, the interface is not required and removing it can avoid
virtual calls.
Differential Revision: https://phabricator.services.mozilla.com/D197142
Make it be output-only, not having that confusing in-out tab-index
parameter that is special for XUL to become focusable with
-moz-user-focus: normal. Instead, do that explicitly in
nsIFrame::IsFocusable().
Also, call it IsFocusableWithoutStyle(), since that's what it is.
Differential Revision: https://phabricator.services.mozilla.com/D195644
This implements the focus behavior described in [1]:
1. Moves focus from an invoking element to its invoked popover,
regardless of where in the DOM that popover lives.
2. Moves focus back to the next focusable element after the
invoking element once focus leaves the invoked popover.
3. Skips over an open invoked popover otherwise.
[1] https://html.spec.whatwg.org/#focus-navigation-scope-owner
Differential Revision: https://phabricator.services.mozilla.com/D190560
This implements the focus behavior described in [1]:
1. Moves focus from an invoking element to its invoked popover,
regardless of where in the DOM that popover lives.
2. Moves focus back to the next focusable element after the
invoking element once focus leaves the invoked popover.
3. Skips over an open invoked popover otherwise.
[1] https://html.spec.whatwg.org/#focus-navigation-scope-owner
Differential Revision: https://phabricator.services.mozilla.com/D190560
This implements the focus behavior described in [1]:
1. Moves focus from an invoking element to its invoked popover,
regardless of where in the DOM that popover lives.
2. Moves focus back to the next focusable element after the
invoking element once focus leaves the invoked popover.
3. Skips over an open invoked popover otherwise.
[1] https://html.spec.whatwg.org/#focus-navigation-scope-owner
Differential Revision: https://phabricator.services.mozilla.com/D190560
This implements the focus behavior described in [1]:
1. Moves focus from an invoking element to its invoked popover,
regardless of where in the DOM that popover lives.
2. Moves focus back to the next focusable element after the
invoking element once focus leaves the invoked popover.
3. Skips over an open invoked popover otherwise.
[1] https://html.spec.whatwg.org/#focus-navigation-scope-owner
Differential Revision: https://phabricator.services.mozilla.com/D190560
This implements the focus behavior described in [1]:
1. Moves focus from an invoking element to its invoked popover,
regardless of where in the DOM that popover lives.
2. Moves focus back to the next focusable element after the
invoking element once focus leaves the invoked popover.
3. Skips over an open invoked popover otherwise.
[1] https://html.spec.whatwg.org/#focus-navigation-scope-owner
Differential Revision: https://phabricator.services.mozilla.com/D190560
This patch shouldn't change behavior. It just removes a lot of noise
from that function.
In particular, it simplifies various conditions that can't happen, like
startNode being a text-node but a form control at the same time, or a
text-node and the root element.
Differential Revision: https://phabricator.services.mozilla.com/D186964
We have more readable and faster versions (that just omit the namespace
arg).
Mostly done via sed, with a couple helpers to use the faster lookups
where possible.
Differential Revision: https://phabricator.services.mozilla.com/D181795
