1634556 - Ctrl+scroll wheel does not zoom in web extension panels or popups

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P1)

Description

[alangoodkin]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

View or start composing a message. Attempt to zoom in or out using Ctrl+mouse wheel has no effect.

Actual results:

Nothing happened. Same in Safe mode.

Ctrl-"+", Ctrl+"-" and Ctrl-0 behave properly.

I am using Windows 10. I have not tried to zoom in Thunderbird using Ctrl-scroll wheel before now, but scrolling in Thunderbird works properly, and zooming with Ctrl-wheel works in other apps, including Firefox v75 and earlier.

Expected results:

The message should have zoomed in or out.

Comment 1

[Alice0775 White]

Reproduced on Tb77.0a1 windows10.

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=a2cc7c9b4a0635baabcb037cc89c1c5a59f1d4fa&tochange=6c2764c30d1636e8e28af1670e80ae8fa3424ed5
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fc760793ad44d712a066265c6c11b82aad450055&tochange=fa76ecb197c0fb0c699e2173d352059970d65545

Regressed by:
0a997b81370da1ef5ba34cf6f3bef985784903f0 Brad Werth — Bug 1516413 Part 3: Update a zoom test to wait on the outcome, not the trigger. r=smaug
41e64804283ad766767c01aaac2b765d327d787d Brad Werth — Bug 1516413 Part 2: Make browser FullZoom act on the new zoom change events. r=mstange
f43efc0c67bdffaf9d31308cffde3c4c0b7ea339 Brad Werth — Bug 1516413 Part 1: Make EventStateManager zoom delta functions send chrome events instead of acting directly. r=mstange

Comment 2

[Brad Werth [:bradwerth]]

Along the way of solving Bug 1516413, I proposed a way to enable control-wheel scrolling for non-browser panels in https://phabricator.services.mozilla.com/D59259?id=219448. That version of the patch attempted to identify webextensions only, was faulty, and I took it out in the next version of the patch. To solve this bug, we'll need something similar that can identify all the non-browser panel cases that should allow control-wheel zooming.

Markus, can you propose a method to identify the appropriate class of panels? That would include Thunderbird panels, and web extension panels. Something that could be checked within the context of the EventStateManager::ChangeZoom function? If you can do that, I should be able to resurrect the old code into something that will solve this bug.

Comment 3

[Markus Stange [:mstange]]

I'm not sure. Rather than making EventStateManager act differently depending on the context, it might be better to just handle the DOM events you added in all the places that want zoomable browsers.
That might lead to more code duplication, though. Maybe there's a good place to share the code... but I don't know where that would be. Can you ask somebody who has more experience with front-end code around <browser>s?

Comment 4

[Firefox Bug Husbandry Bot]

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is -- (non,) indicating it has has not been previously triaged, the bug's Severity is being updated to -- (default, untriaged.)

Comment 5

[Magnus Melin [:mkmelin]]

Brad, any update?

Comment 6

[Brad Werth [:bradwerth]]

Needinfo'ing myself to get an answer to this.

Comment 7

[Brad Werth [:bradwerth]]

Since my patches landed in regressing Bug 1516413, the ZoomActor has been removed in Bug 1634218. I don't think that closes off the option of going back to the old way of doing things (directly manipulating the zoom level of the ContentViewer), but it gives me some pause on how to fix this the right way. I'm going to try to follow the advice in comment 3 and make more embedding elements respond to the DoZoomEnlargeBy10/DoZoomReduceBy10 events. I've confirmed that CanonicalBrowsingContext::DispatchWheelZoomChange is hit for the sidebar views, and that a XULFrameElement is the embedding element in that case. The XUL element is hosting chrome://browser/content/webext-panels.xhtml in that case. I'll try to add event listeners there and uplevel as far as I need to in order to capture all non-browser views.

Comment 9

[Brad Werth [:bradwerth]]

Attachment: Bug 1634556 Part 1: Make web extension sidebars listen to zoom enlarge/reduce events.

These events are fired for all parent process documents by the code that
handles native mousewheel events. This change adds listeners to those events
and handles them similarly to how they are handled for browsers.

Comment 10

[Brad Werth [:bradwerth]]

(In reply to Brad Werth [:bradwerth] from comment #9)

Created attachment 9165797 [details]
Bug 1634556: Make web extension panels listen to zoom enlarge/reduce events.

These events are fired for all parent process documents by the code that
handles native mousewheel events. This change adds listeners to those events
and handles them similarly to how they are handled for browsers.

I honestly don't know if this will address the problem in Thunderbird. I'm not sure how it's constructed -- does it use webext-panels.xhtml? I tested this with the Notes web extension, and it works there. I may need to add another such fix for Thunderbird. Can a Thunderbird peer help test this or give insight into how the panels in TB are hosted (which URLs)?

Comment 11

[Richard Marti (:Paenglab)]

Magnus, could you help here?

Comment 12

[Magnus Melin [:mkmelin]]

Thunderbird doesn't use webext-panels.xhtml. So it doesn't look like the patch would change/fix any code that Thunderbird uses.

Thunderbird doesn't use e10s yet. The compose content is in an <editor> https://searchfox.org/comm-central/rev/eb69eff3ca16509af6cf8f2dd29be396bebbc14d/mail/components/compose/content/messengercompose.xhtml#2526 and the viewing content in a <browser> https://searchfox.org/comm-central/rev/eb69eff3ca16509af6cf8f2dd29be396bebbc14d/mail/base/content/messenger.xhtml#769

I see we have a forked ZoomManager. That might need needs adjustments - https://searchfox.org/comm-central/search?q=ZoomManager&path=

Comment 13

[Brad Werth [:bradwerth]]

(In reply to Magnus Melin [:mkmelin] from comment #12)

Thunderbird doesn't use webext-panels.xhtml. So it doesn't look like the patch would change/fix any code that Thunderbird uses.

Thunderbird doesn't use e10s yet. The compose content is in an <editor> https://searchfox.org/comm-central/rev/eb69eff3ca16509af6cf8f2dd29be396bebbc14d/mail/components/compose/content/messengercompose.xhtml#2526 and the viewing content in a <browser> https://searchfox.org/comm-central/rev/eb69eff3ca16509af6cf8f2dd29be396bebbc14d/mail/base/content/messenger.xhtml#769

I see we have a forked ZoomManager. That might need needs adjustments - https://searchfox.org/comm-central/search?q=ZoomManager&path=

If there's a Thunderbird developer who is prepared to do the work, the D84767 patch shows what needs to be done:

1. On the parent process, listen for the "DoZoomEnlargeBy10" and ""DoZoomReduceBy10" events, and either set browser.fullZoom (if you have a browser) or docShell.zoom in increments of 0.1.
2. Cap the zoom levels based on the prefs "zoom.minPercent" and "zoom.maxPercent". The D84767 patch gets these values from ZoomManager, but that's not strictly necessary, and they can be retrieved using more direct means.

Comment 14

[Magnus Melin [:mkmelin]]

Split it off into bug 1655244. I guess this bug should move to some other component?

Comment 15

[Brad Werth [:bradwerth]]

(In reply to Magnus Melin [:mkmelin] from comment #14)

Split it off into bug 1655244. I guess this bug should move to some other component?

Thank you. I'll mark this as a WebExtensions issue, and see if we can land the existing patch.

Comment 16

[Brad Werth [:bradwerth]]

Attachment: Bug 1634556 Part 4: Add tests of mousewheel zoom in a sidebar, browser action, and page action panels.

Depends on D84767

Comment 17

[Shane Caraveo (:mixedpuppy)]

Did this regress the browser and page action panels, or just the sidebar? I commented on the review that the patch only works with the sidebar.

Comment 18

[Brad Werth [:bradwerth]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #17)

Did this regress the browser and page action panels, or just the sidebar? I commented on the review that the patch only works with the sidebar.

I'm sorry, I'm not familiar enough with the different panel types, embedding methods, to confirm that this works in all cases. As demonstrated in the test, this works for web extensions embedded in a sidebar. If there is a case that fails, please provide Steps to Reproduce and I will attempt to make the patch work correctly in that case as well.

To be clear, mousewheel zooming already works correctly for browser panels and for web extensions hosted in their own tab, and with Bug 1655244, it will work in Thunderbird. I'm not sure what other cases need to be covered.

Comment 19

[Shane Caraveo (:mixedpuppy)]

Attachment: datalist.xpi

This is an addon is use at times for various manual testing. It has a sidebar, page and browser action. To see the page action, you'll need to go to any website (not an about:* page). With this addon, I could see that the zoom worked in the sidebar, but not in either the page or browser action.

Comment 20

[Brad Werth [:bradwerth]]

Thank you! I see how to trigger the page action and browser action with this example. I'll try to figure out how to make zoom work in these cases as well.

Comment 21

[Brad Werth [:bradwerth]]

Attachment: Bug 1634556 Part 2: Make web extension popup panels listen to zoom enlarge/reduce events.

These events are fired for all parent process documents by the code that
handles native mousewheel events. This change adds listeners to those events
and handles them similarly to how they are handled for browsers.

Depends on D84767

Comment 22

[Brad Werth [:bradwerth]]

Attachment: Bug 1634556 Part 3: Make web extension popups choose sizes based on zoom levels.

Now that extension popups can have zoom levels other than 100%, this code
ensures that the sizes chosen for the popups is scaled based on the zoom
level.

Depends on D87113

Comment 23

[Brad Werth [:bradwerth]]

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

Comment 24

[Brad Werth [:bradwerth]]

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

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3224b75754e5
Part 1: Make web extension sidebars listen to zoom enlarge/reduce events. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/2ba70048753b
Part 2: Make web extension popup panels listen to zoom enlarge/reduce events. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/c0a31a8d4f7e
Part 3: Make web extension popups choose sizes based on zoom levels. r=extension-reviewers,mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/735520d80231
Part 4: Add tests of mousewheel zoom in a sidebar, browser action, and page action panels. r=mixedpuppy

Comment 26

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/3224b75754e5
https://hg.mozilla.org/mozilla-central/rev/2ba70048753b
https://hg.mozilla.org/mozilla-central/rev/c0a31a8d4f7e
https://hg.mozilla.org/mozilla-central/rev/735520d80231