Closed Bug 1634556 Opened 1 year ago Closed 8 months ago

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

Categories

(WebExtensions :: General, defect, P1)

76 Branch
defect

Tracking

(thunderbird_esr78 affected, firefox-esr78 wontfix, firefox82 wontfix, firefox83 wontfix, firefox84 fixed)

RESOLVED FIXED
84 Branch
Tracking Status
thunderbird_esr78 --- affected
firefox-esr78 --- wontfix
firefox82 --- wontfix
firefox83 --- wontfix
firefox84 --- fixed

People

(Reporter: lynandalan, Assigned: bradwerth)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(5 files)

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.

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

Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Regressed by: 1516413
No longer regressed by: 1516413
Regressed by: 1516413
Component: Untriaged → Mail Window Front End

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.

Flags: needinfo?(mstange)
Assignee: nobody → bwerth

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?

Flags: needinfo?(mstange)

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.)

Severity: normal → --

Brad, any update?

Priority: -- → P1

Needinfo'ing myself to get an answer to this.

Flags: needinfo?(bwerth)

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.

Duplicate of this bug: 1654830

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.

Flags: needinfo?(bwerth)

(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)?

Flags: needinfo?(lynandalan)

Magnus, could you help here?

Flags: needinfo?(mkmelin+mozilla)

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=

Flags: needinfo?(mkmelin+mozilla)

(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.

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

(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.

Component: Mail Window Front End → General
Flags: needinfo?(lynandalan)
Product: Thunderbird → WebExtensions
Summary: Ctrl+scroll wheel does not zoom in message reader, message pane or compose window → Ctrl+scroll wheel does not zoom in web extension panels
Version: 76 → 76 Branch
Blocks: 1655223
Attachment #9165797 - Attachment description: Bug 1634556: Make web extension panels listen to zoom enlarge/reduce events. → Bug 1634556 Part 1: Make web extension panels listen to zoom enlarge/reduce events.

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.

Flags: needinfo?(bwerth)

(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.

Flags: needinfo?(bwerth)
Attached file 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.

Flags: needinfo?(bwerth)

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.

Flags: needinfo?(bwerth)
Summary: Ctrl+scroll wheel does not zoom in web extension panels → Ctrl+scroll wheel does not zoom in web extension panels or popups
Attachment #9165797 - Attachment description: Bug 1634556 Part 1: Make web extension panels listen to zoom enlarge/reduce events. → 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.

Depends on D84767

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

Attachment #9168964 - Attachment description: Bug 1634556 Part 2: Add a test of mousewheel zoom in a sidebar webextension. → Bug 1634556 Part 4: Add a test of mousewheel zoom in a sidebar webextension.
Attachment #9168964 - Attachment description: Bug 1634556 Part 4: Add a test of mousewheel zoom in a sidebar webextension. → Bug 1634556 Part 4: Add tests of mousewheel zoom in a sidebar, browser action, and page action panels.
Pushed by bwerth@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.