Implement the 'interactive-widget' meta viewport key, to allow controlling viewport resize behaviour when showing a virtual keyboard
Categories
(Core :: Layout: Scrolling and Overflow, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox131 | --- | fixed |
People
(Reporter: botond, Assigned: hiro)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-complete)
Attachments
(14 files, 2 obsolete files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
Bug 1831649 - Tweak the visual viewport size for resizes-visual and overlays-content modes. r?botond
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
384 bytes,
text/html
|
Details | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
46.98 KB,
image/png
|
Details |
Filing this bug to track a Firefox implementation of the interactive-widget
meta viewport key, as a mechanism for allowing page authors to control the viewport resize behaviour when showing a virtual keyboard (or potentially other interactive widgets).
A few relevant links:
- Explainer doc for proposal: https://github.com/bramus/viewport-resize-behavior/blob/main/explainer.md#proposal
- CSSWG tracking issue: https://github.com/w3c/csswg-drafts/issues/7767
- Mozilla standards-position ticket: https://github.com/mozilla/standards-positions/issues/693
My understanding is that this is a newer alternative proposal to the Virtual Keyboard API tracked in bug 1730568.
Assignee | ||
Comment 1•1 year ago
|
||
test_event_target_radius.html doesn't require showing the software keyboard.
The keyboard shown by the test persists there for some reasons even after the
test has finished. Thus a new mochitest which will be introduced in this commit
series will not properly run, the new test needs to start without the software
keyboard.
Updated•1 year ago
|
Assignee | ||
Comment 2•1 year ago
|
||
It's defined in the CSS viewport spec.
https://drafts.csswg.org/css-viewport/#interactive-widget-section
Assignee | ||
Comment 3•1 year ago
|
||
Assignee | ||
Comment 4•1 year ago
|
||
Assignee | ||
Comment 5•1 year ago
|
||
So that even if the chrome window gets resized by the software keyboard, the
contents can be laid out as if there's no software keyboard for overlays-content
and resizes-visual modes.
Assignee | ||
Comment 6•1 year ago
|
||
|aDisplaySize| is based on the document viewer size which is changed by the
software keyboard, thus to make the visual viewport size unchange for
overlays-content mode we need to compensate the keyboard size.
Assignee | ||
Comment 7•1 year ago
|
||
Without this reflow changing the interactive-widget value will do nothing.
Though it's a rare case, we do it in a mochitest.
NOTE: The actual route to kick a reflow when the value is changed is;
- MobileViewportManager is listening meta viewport changes
- the listener invokes MobileViewportManager::RefreshViewportSize
Assignee | ||
Comment 8•1 year ago
|
||
A caveat; test_interactive_widget.html changes the top level content document so
that after the all test cases finished the changes need to be reverted.
The reason why we take this approach rather then opening a new tab is that on
the GeckoView test runner the software keyboard never appears in newly opened
documents. Newly opened documents on the GeckoView test runner are active but
background, it's a weird setup, I haven't found out any ways to make the
software keyboard work there without any troubles.
Assignee | ||
Comment 9•1 year ago
|
||
And add a caveat about windowSoftInputMode in a document.
We implement interactive-widget [1] in a way that the content is laid out as if
the window doesn't get resized even if the application window gets resized by
the software keyboard. To make this way work, using adjustResize
is mandatory.
An alternative approach to implement it is to change windowSoftInputMode value
depending on each content's interactive-widget value in each Android Activity,
i.e., set to adjustResize
on interactive-widget=resizes-content
mode, set to
adjustUnspecified
on interactive-widget=resizes-visual
or on
interactive-widget=overlays-mode
. It will force applications to do more work.
[1] https://drafts.csswg.org/css-viewport/#interactive-widget-section
Assignee | ||
Comment 10•1 year ago
|
||
Updated•10 months ago
|
Updated•10 months ago
|
Updated•10 months ago
|
Assignee | ||
Comment 11•10 months ago
|
||
On Android there's no window-ed mode so that even if those size are not in sync,
it will not cause any problems at least the visible size is larger than nsView
size. Though I am not 100% confident whether there's any exception or not.
Assignee | ||
Comment 12•10 months ago
|
||
Updated•10 months ago
|
Updated•10 months ago
|
Assignee | ||
Comment 13•10 months ago
|
||
Assignee | ||
Comment 14•10 months ago
|
||
Jon or Titouan, I need you guys help.
One of the patches (D204164) here uses ViewCompat.setOnApplyWindowInsetsListener, it works perfectly on GeckoView example. But it doesn't work on Fenix.
The reason is that one of our android-components also uses ViewCompat.setOnApplyWindowInsetsListener with null listener in Activity.exitImmersiveMode
and unfortunately Activity.exitImmersiveMode
gets called after GeckoView.onAttachedToWindow
gets called where I tried to use ViewCompat.setOnApplyWindowInsetsListener
(you can checked it in D204164). That means, the listener I attached is nullified soon after its registration. How can I solve this conflict? One odd thing I've observed is that Activity.exitImmersiveMode
gets called without any Activity.enterImmersiveMode
, I suppose they should be paired.
Comment 15•10 months ago
|
||
Hi Hiro! I'll take some time to look into this on Thursday.
Comment 16•10 months ago
|
||
Back from PTO - clearing NI to let Titouan respond.
Comment 17•10 months ago
|
||
The implementation is obviously ongoing and interative-widget
has already been added to the spec. in the meantime.
Though I want to point out that someone noted on the related GitHub issue that there doesn't seem to be a resolution yet for adding this feature. And nobody has answered that comment so far.
Emilio, as one of the authors of the spec., I guess it's on you to clarify that.
Regarding documentation, once this lands, the page https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta/name#standard_metadata_names_defined_in_other_specifications needs to be updated and probably reworked as well as the related browser compat data (which currently doesn't include any of the viewport keys). As there are a bunch of keys already, I guess it also makes sense to additionally create a separate page for <meta name="viewport">
under https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta/name/viewport.
Sebastian
Updated•10 months ago
|
Updated•10 months ago
|
Comment 19•10 months ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #14)
Jon or Titouan, I need you guys help.
One of the patches (D204164) here uses ViewCompat.setOnApplyWindowInsetsListener, it works perfectly on GeckoView example. But it doesn't work on Fenix.
The reason is that one of our android-components also uses ViewCompat.setOnApplyWindowInsetsListener with null listener in
Activity.exitImmersiveMode
and unfortunatelyActivity.exitImmersiveMode
gets called afterGeckoView.onAttachedToWindow
gets called where I tried to useViewCompat.setOnApplyWindowInsetsListener
(you can checked it in D204164). That means, the listener I attached is nullified soon after its registration. How can I solve this conflict? One odd thing I've observed is thatActivity.exitImmersiveMode
gets called without anyActivity.enterImmersiveMode
, I suppose they should be paired.
Sorry for the delay, I finally took time to investigate the ViewCompat.setOnApplyWindowInsetsListener
issue.
I haven't figured out a good way to solve it. There's a conflict between Fenix and GeckoView applying a listener on the same view, with indeed Fenix reseting the listener to null in Activity.exitImmersiveMode
. Activity.exitImmersiveMode
gets called "preventively" at several times, without checking if the app was in immersive mode before. We could try to check if immersive mode is on
before applying the code to exit from it, but I'd say it would be hard to make sure we don't leave any corner case behind.
I wonder if it would be possible to expose an API from GeckoView, like windowInsetsChanged(...)
, so that Fenix would be responsible to tell GeckoView about the changes. It would make it easier to handle the conflict, as Fenix would handle both the code related to exitImmersiveMode
and to the keyboard resizing.
I'm not 100% sure it would be the best option, but for now I couldn't come up with a better idea. Any thought?
Assignee | ||
Comment 20•9 months ago
|
||
(In reply to Titouan Thibaud [:tthibaud] from comment #19)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #14)
Jon or Titouan, I need you guys help.
One of the patches (D204164) here uses ViewCompat.setOnApplyWindowInsetsListener, it works perfectly on GeckoView example. But it doesn't work on Fenix.
The reason is that one of our android-components also uses ViewCompat.setOnApplyWindowInsetsListener with null listener in
Activity.exitImmersiveMode
and unfortunatelyActivity.exitImmersiveMode
gets called afterGeckoView.onAttachedToWindow
gets called where I tried to useViewCompat.setOnApplyWindowInsetsListener
(you can checked it in D204164). That means, the listener I attached is nullified soon after its registration. How can I solve this conflict? One odd thing I've observed is thatActivity.exitImmersiveMode
gets called without anyActivity.enterImmersiveMode
, I suppose they should be paired.Sorry for the delay, I finally took time to investigate the
ViewCompat.setOnApplyWindowInsetsListener
issue.I haven't figured out a good way to solve it. There's a conflict between Fenix and GeckoView applying a listener on the same view, with indeed Fenix reseting the listener to null in
Activity.exitImmersiveMode
.Activity.exitImmersiveMode
gets called "preventively" at several times, without checking if the app was in immersive mode before. We could try to check if immersive mode ison
before applying the code to exit from it, but I'd say it would be hard to make sure we don't leave any corner case behind.
Yeah, I agree.
I wonder if it would be possible to expose an API from GeckoView, like
windowInsetsChanged(...)
, so that Fenix would be responsible to tell GeckoView about the changes. It would make it easier to handle the conflict, as Fenix would handle both the code related toexitImmersiveMode
and to the keyboard resizing.
An annoying point here is that GeckoView is just a View, not an application (I should use Activity in terms of Android development?). It would also make things awkward if GeckoView forces every application (Activity) to do something for the application global things? For example, if GeckoView provided the API to be notified WindowInsets change for the application, it will be a static global function looks like;
static public void GeckoView.addWindowInsetsListener(final Activity activity, OnApplyWindowInsetsListener listener);
And a call site of ViewCompat.setOnApplyWindowInsetsListener here will be GeckoView.addWindowInsetsListener(this, ...)
.
Is it okay? I would say that an ideal way of providing such APIs is probably to create a new repo for it, named Android ViewCompat utils or some such, then use the repo both in GeckoView and android-components. But it's somewhat opposed to our recent Fenix repo integration into mozilla-central. :/
I'm not 100% sure it would be the best option, but for now I couldn't come up with a better idea. Any thought?
Anyways, I have no great idea to solve this conflict, so tried to implement the API in GeckoView and it works as expected. I will upload it soon. Thanks!
Assignee | ||
Comment 21•9 months ago
|
||
Assignee | ||
Comment 22•9 months ago
|
||
Updated•9 months ago
|
Updated•9 months ago
|
Comment 23•8 months ago
|
||
Comment 24•8 months ago
•
|
||
Backed out for causing multiple failures.
- Push with failures - AC-android-all opt
- Failure Log
- Failure line: file:///builds/worker/checkouts/gecko/mobile/android/android-components/components/ui/widgets/src/test/java/mozilla/components/ui/widgets/behavior/EngineViewScrollingBehaviorTest.kt:566:10: error: Class 'DummyEngineView' is not abstract and does not implement abstract member public abstract fun addWindowInsetsListener(key: String, listener: OnApplyWindowInsetsListener?): Unit defined in mozilla.components.concept.engine.EngineView
- Push with failures - focus-android-all opt
- Failure Log
- Failure line: file:///builds/worker/checkouts/gecko/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/BrowserFragmentTest.kt:78:1: error: Class 'DummyEngineView' is not abstract and does not implement abstract member public abstract fun addWindowInsetsListener(key: String, listener: OnApplyWindowInsetsListener?): Unit defined in mozilla.components.concept.engine.EngineView
- Push with failures - android wpt failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | /webdriver/tests/classic/element_click/scroll_into_view.py | test_partially_visible_does_not_scroll[9] - assert (490, 681) == approx((490 ± 1.0e+00, 1386 ± 1.0e+00))
- Push with failures - fenix-ui-test opt
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | org.mozilla.fenix.ui.HistoryTest#verifyHistoryMenuWithHistoryItemsTest | androidx.test.espresso.PerformException: Error performing 'single click - At Coordinates: 1032, 1846 and precision: 16, 16' on view 'view.getContentDescription() is "Menu"'.
Also, please check this mochitests failure.
Assignee | ||
Comment 25•7 months ago
|
||
I've fixed the first two failure in comment 24.
For the wpt one, I can't run it locally due to bug 1908788.
For the fenix-ui-test one, I can't find any ways to run it locally, I've asked it in #Fenix, but I haven't got responses yet.
For the mochitest failure, I couldn't run it yesterday because of bug 1908713, now I can run it, but can't reproduce the failure locally. :/
Assignee | ||
Comment 26•7 months ago
|
||
For the wpt failure, the culprit is D204165 which just added android:windowSoftInputMode="stateUnspecified|adjustResize"
to the TestRunner activity. The fact makes me suspect the test itself should have been unstable if we did run the test on GeckoView example or Fenix. :/
Comment 27•7 months ago
•
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #25)
For the fenix-ui-test one, I can't find any ways to run it locally, I've asked it in #Fenix, but I haven't got responses yet.
#mobile-android-team (Slack) is where the team lives
Hi Hiroyuki, you will need to build Fenix, we have documentation here (Kotlin Espresso Tests) that run on a connected device (physical/emulator) either through Gradle/Android Studio:
https://firefox-source-docs.mozilla.org/mobile/android/fenix/UI-Tests.html
They can also be scheduled with ./mach try --preset firefox-android
which will run {Fenix/AC/Focus} these suits on Firebase Test Lab on a variety of devices.
Matrix: https://console.firebase.google.com/project/moz-fenix/testlab/histories/bh.66b7091e15d53d45/matrices/8459306338494513927/details?stepId=bs.3bc345a966ef66d5&testCaseId=1 (videos/logs/screenshots/test results)
As this broke a vast (~290 tests) tests prior to back-out in our suite, let's ensure these changes are tested. Please reach out for further questions.
For what it's worth it looks like the vast majority are hitting an issue where the address-bar is merged with the system UI (see attachment), viewport all wonky. A large number of our tests confirm the current URL and it can't detect it behind the system UI.
Comment 28•7 months ago
|
||
Assignee | ||
Comment 29•7 months ago
|
||
Thanks Aaron. Yeah, I had read the document and had built Fenix locally, but the command in the document, "/gradlew blah" didn't work locally. That's way I did ask.
Anyways, the screenshot is quite helpful to tell what's going on. It makes me suspect the recent Fenix's navbar change did something I wasn't expecting. All changesets here had done before the navbar change. :/
Assignee | ||
Comment 30•7 months ago
|
||
FWIW, the culprit caused fenix-ui-test failure is D210894. Odd.
Assignee | ||
Comment 31•7 months ago
|
||
I've fixed all failure in comment 24 other than the (intermittent) mochitest failure. That's said, while I was checking whether there's any other failure or not, I realized there's a perma failure in screenshots-arm job and interestingly it fails on m-c.
Aaron, is it worth filing a bug? Or it's a known failure?
Comment 32•7 months ago
•
|
||
Thanks. I filed a bug about it.
For the Try
above, using Fuzzy query=fenix arm
did not schedule a complete UI test run. Can you run ./mach try --preset firefox-android
to ensure a run against Fenix/Focus/AC is scheduled? Presets found in https://searchfox.org/mozilla-central/source/tools/tryselect/try_presets.yml#134
Assignee | ||
Comment 33•7 months ago
|
||
I figured out how the test_interactive_widget.html fails.
The test inserts a meta viewport tag in the top level content document at the beginning of the test, then in the first test case (for interactive-widget=resizes-content
) we wait for a resize event triggered by appearing the software keyboard. But in a race condition, a resize event triggered by the meta viewport tag insertion is received while we are waiting for a resize event in the first test.
Assignee | ||
Comment 34•7 months ago
|
||
Now I've addressed all failure cases I know of here, I am waiting for reviewing bug 1908828.
Comment 35•7 months ago
|
||
Comment 36•7 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a39d8020f6ed
https://hg.mozilla.org/mozilla-central/rev/60c098037541
https://hg.mozilla.org/mozilla-central/rev/94b7afd117cb
https://hg.mozilla.org/mozilla-central/rev/bfbf94d9ecbc
https://hg.mozilla.org/mozilla-central/rev/5717fc0000f5
https://hg.mozilla.org/mozilla-central/rev/8b197fb54abc
https://hg.mozilla.org/mozilla-central/rev/2b6b61d3db94
https://hg.mozilla.org/mozilla-central/rev/1a16365ecd2c
https://hg.mozilla.org/mozilla-central/rev/9fa674dd294e
https://hg.mozilla.org/mozilla-central/rev/ee42ede208d0
https://hg.mozilla.org/mozilla-central/rev/951c1ac69475
https://hg.mozilla.org/mozilla-central/rev/fae4fd3feb6b
Comment 37•5 months ago
•
|
||
I have tested this on Firefox Nightly for Android v132.0a1 and it does not behave in the same manner as Chrome for Android.
I tested this with the demo built by Bramus, I found on this post Prepare for viewport resize behaviour
Can we confirm that this is working as expected.
Assignee | ||
Comment 38•5 months ago
|
||
Thanks for testing, Dave.
Though I haven't checked your screenshots in detail, indeed the screenshot with interactive-widget=resizes-visual
on Firefox looks wrong. That's said it's totally different from what I am seeing on my pixel 3. On my pixel 3 the bottom line of the blue dotted line is not visible initially, i.e. the layout viewport isn't resized with interactive-widget=resizes-visual
.
I can't immediately figure out why it doesn't work on your environment. Can please you file a new bug? Thanks!
Assignee | ||
Comment 39•5 months ago
|
||
It would be nice to have your about:support result in the new bug. The only one I am wondering now is the Android version, though.
Comment 40•5 months ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #39)
It would be nice to have your about:support result in the new bug. The only one I am wondering now is the Android version, though.
Sorry for the delay I've been away.
I shall get on this now
Comment 41•5 months ago
|
||
(In reply to Dave Letorey from comment #40)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #39)
It would be nice to have your about:support result in the new bug. The only one I am wondering now is the Android version, though.
Sorry for the delay I've been away.
I shall get on this now
Comment 42•3 months ago
|
||
Description
•