The fixed -400px horizontal offset used by
scrollIntoView led to horizontal scroll only moving
part-way right on narrow screens. The highlights near
the right-edge remained party or completely off
screen.
This centres the highlighted match on any viewport width while
clamping the left margin to 20-400px. On very narrow screens
the scrollbar now moves all the way to the right instead of
stopping midway.
After clicking the find button we need to wait for the input field to
appear before we try to type in it, but instead we were waiting for the
button to appear. However, the button is always present, so the current
`waitForSelector` call is basically a no-op, and this can cause the
integration tests to fail intermittently if we continue before the
input field is actually visible.
This appears to be due a typo first introduced in commit
d10da907dac86c103b787127110a59aed82c7aaa that has been copy/pasted.
This commit fixes the issue by waiting for the input field to be visible
before we continue typing in it.
To avoid being able to introduce dependencies between tests this commit
makes sure that we close the document between tests so that we can't
accidentally rely on state set by a previous test.
When computing the left offset of the highlighted text, we cannot use
.offsetLeft because the text might have been scaled through CSS, and it
needs to be taken into account.
Use `.getClientRects()`/`.getBoundingClientRect()` instead, which will
return measurements scaled appropriately.
The first goal of this patch was to remove the tabindex because it helps
to improve overall a11y. That led to move some html elements associated
with the buttons which helped to position these elements relatively to their
buttons.
Consequently it was easy to change the toolbar height (configurable in Firefox
with the pref browser.uidensity): it's the second goal of this patch.
For a11y reasons we want to be able to change the height of the toolbar to make
the buttons larger.
This commit implements the following improvements for the test:
- Replace the hardcoded highlight padding with a dynamic calculation. It
turns out that the amount of padding can differ per system, possibly
due to e.g. local (font) settings, which could cause the test to fail.
The test now no longer implicitly depends on the environment.
- Remove the page offset subtraction. It's only needed for one assertion,
and we can simply add the page offset there once.
- Improve the "magic" numbers in the test. The number 5 is removed now
that the padding calculation made it obsolete, the number 28 is changed
to 30 because that's the actual value in the PDF data and the number 4
is explained a bit more to state that the space also counts as a glyph.
- Annotate the steps and checks to improve readability of the test and
to explain why certain calculations have to be performed.
- For the generic viewer we use @fluent/dom and @fluent/bundle
- For the builtin pdf viewer in Firefox, we set a localization url
and then we rely on document.l10n which is a DOMLocalization object.
The current test fails intermittently only on Windows for unknown
reasons: the code is correct and on Linux it always passes. However, we
have already spent quite a lot of time on this test, so rather than
spending even more time on it I figured we should look at what behavior
the test is trying to check and find an alternative way to do it that
can't trigger this intermittent issue anymore.
This commit changes the test to use a term that only exists once in the
entire document so we cannot accidentally highlight another match
anymore. This doesn't change anything about the behavior that this test
aims to check: we still test searching in the XFA layer, we still test
that the original term is matched case-insensitively and we still test
that that match is actually highlighted. Note that the only objective of
the test is confirming that the search functionality covers the XFA
layer, so the exact phrase/match is not the interesting bit.
