Closed Bug 1551302 (visual-viewport-api-desktop) Opened 2 years ago Closed 4 months ago

Enable the Visual Viewport API on desktop

Categories

(Core :: Panning and Zooming, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
91 Branch
Tracking Status
relnote-firefox --- 91+
firefox68 --- wontfix
firefox91 --- fixed

People

(Reporter: botond, Assigned: tnikkel)

References

(Depends on 1 open bug, Blocks 2 open bugs, Regressed 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

In bug 1512813, we have enabled the Visual Viewport API by default on Android.

This bug tracks also enabling it by default on desktop, which depends on (and should be released concurrently with) desktop zooming.

Type: defect → enhancement
See Also: → visual-viewport-api

This should probably block desktop zooming riding the trains to release.

Alias: visual-viewport-api-desktop

(Also removing the desktop-zooming-xp dependency since I believe desktop zooming now works well enough that work on this is unblocked. If we discover specific open dependencies we can add them individually.)

No longer depends on: desktop-zoom-xp

Looks like only unrelated intermittents, so we could flip the pref now if we wanted.

I wonder if we should also be dropping the meta-viewport pref from https://searchfox.org/mozilla-central/source/testing/web-platform/meta/css/css-device-adapt/__dir__.ini and https://searchfox.org/mozilla-central/source/testing/web-platform/meta/visual-viewport/__dir__.ini. But that could be deferred to a follow-up.

If we do flip the pref (I'm in favour of doing it), we should send an intent email to dev-platform first.

Marking all the tests as passing on desktop that were only marked as passing on android in the visual-viewport directory looks like we have one failure

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4b0fe30c51d874c5982df3260eedc1f19c325ea

TEST-UNEXPECTED-FAIL | /visual-viewport/viewport-resize-event-on-load-overflowing-page.html | Resize event fired exactly once against window.visualViewport if scrollbars affect layout. - assert_equals: expected 1 but got 3

The test also fails when I run it locally with overlay scrollbars, this time we get 1 resize event and 0 are expected.

@martin: Do you have an overview on how much the visual viewport api is used and what the priority of this would be for release.

Flags: needinfo?(mbalfanz)

Thanks for bringing this up!

My initial research shows quite a bit of use in production (e.g. fullstory.com and zendesk) and open source (quasar framework, popper.js and more). I want to better understand how it's used to get a feeling for potential webcompat issues if we decide not to ship it. I'll get back to you as soon as I know more, but my gut feeling is that we should be OK without it.

Flags: needinfo?(mbalfanz)
Depends on: 1546387
Severity: normal → N/A
Priority: P3 → P2
Blocks: 1711989

I checked on try we pass all tests now. I changed all test expectations in the directory testing/web-platform/meta/visual-viewport that only expected the test to pass on android to expect it to pass everywhere. This left two annotations

testing/web-platform/meta/visual-viewport/viewport-resize-event-on-load-overflowing-page.html.ini
expected to fail everywhere, bug 1552046 was filed for this already.

testing/web-platform/meta/visual-viewport/viewport-scrollbars-cause-resize.html.ini
expected random for non-debug builds everywhere. Not sure why, seems to be annotated that way just to get the test suite running/green.

Following from comment 5 I also checked that tests pass with dom.metaviewport.enabled set to either the default value or true (like it currently is) in these two directories:

testing/web-platform/meta/css/css-device-adapt/dir.ini
testing/web-platform/meta/visual-viewport/dir.ini

So looks like we are okay to flip the pref from a test perspective.

Martin, any other issue we should investigate before shipping this? (Bug 1711989 is the impetus for doing this now.)

Flags: needinfo?(mbalfanz)

(In reply to Timothy Nikkel (:tnikkel) from comment #10)

Following from comment 5 I also checked that tests pass with dom.metaviewport.enabled set to either the default value or true (like it currently is) in these two directories:

testing/web-platform/meta/css/css-device-adapt/dir.ini
testing/web-platform/meta/visual-viewport/dir.ini

Botond, do you want me to make this change in this bug or a follow up?

Flags: needinfo?(botond)
Assignee: nobody → tnikkel
Status: NEW → ASSIGNED

(In reply to Timothy Nikkel (:tnikkel) from comment #12)

(In reply to Timothy Nikkel (:tnikkel) from comment #10)

Following from comment 5 I also checked that tests pass with dom.metaviewport.enabled set to either the default value or true (like it currently is) in these two directories:

testing/web-platform/meta/css/css-device-adapt/dir.ini
testing/web-platform/meta/visual-viewport/dir.ini

Botond, do you want me to make this change in this bug or a follow up?

I think this bug is fine (unless you anticipate additional fallout and think it's easier to spin it off to avoid blocking this bug).

Flags: needinfo?(botond)
Blocks: 1713094
Pushed by tnikkel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8cda695d8ca8
Enable visual viewport on desktop platforms. r=botond,smaug,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/42b174c62611
Change a few wpt tests to expect pass on desktop. r=botond
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
Duplicate of this bug: 1711989

Sorry for the late response. I don't see any issue in enabling this

Flags: needinfo?(mbalfanz)

Timothy, is that a big enough change to be also added to our consumer release notes in addition to the mention in MDN?

Flags: needinfo?(tnikkel)

Not sure. I asked Botond, he said he didn't think it was big enough for the consumer release notes.

Flags: needinfo?(tnikkel)

On second thought, I see that the 89 release notes have a "Web Platform" section where they mention new web platform APIs. I think we could mention it in that section.

Release Note Request (optional, but appreciated)
[Why is this notable]: It's enabling support for a new web platform API on desktop platforms.
[Affects Firefox for Android]: Firefox for Android already supports this API (since Firefox 68).
[Suggested wording]: "The Visual Viewport API is now supported on desktop platforms."
[Links (documentation, blog post, etc)]: https://developer.mozilla.org/en-US/docs/Web/API/Visual_Viewport_API

relnote-firefox: --- → ?

Note added to nightly 91 release notes.

FYI, FF91 docs work for this can be tracked in https://github.com/mdn/content/issues/6713. There was relatively little to do (remove from experimental pages, add a release note, some minor tidying). I am still waiting on browser compatibility changes to be reviewed: https://github.com/mdn/browser-compat-data/pull/11876

Regressions: 1725569

This was included in the final 91.0 release notes.

You need to log in before you can comment on or make changes to this bug.