886573 - Keyboard zoom controls change shown percentage in Australis zoom widget while in customize mode

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Justin Dolske [:Dolske]]

Seen on user testing. Using the keyboard controls to zoom in/out while in customize mode thankfully doesn't actually zoom the page, but does make the zoom percentage label change in the widget.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

To fix this bug, you should clone the UX branch which is located at https://hg.mozilla.org/projects/ux/. You will need to look at the /browser/components/customizableui/src/CustomizableWidgets.jsm and probably need to update the zoom-controls widget to disregard zoom notification changes while in customization mode.

Comment 2

[Anup Kumar [:reznord]]

Hi, 

I am interested in working on this bug. So please can you assign this bug to me.

Thanks in advance,

Regards,
A.Anup

Comment 3

[Darshana Padmadas]

Attachment: output.patch

Made the necessary changes.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8363005 [details] [diff] [review]
output.patch

Review of attachment 8363005 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/customizableui/src/CustomizableWidgets.jsm
@@ +363,5 @@
>        let window = aDocument.defaultView;
>        function updateZoomResetButton() {
>          //XXXgijs in some tests we get called very early, and there's no docShell on the
>          // tabbrowser. This breaks the zoom toolkit code (see bug 897410). Don't let that happen:
> +        if (document.documentElement.hasAttribute("customizing"))

`document` is not defined here. How did you test this?

Comment 6

[:Gijs (he/him)]

Attachment: make zoom controls display the right thing in Australis even in customize mode,

Darshana, I'm sorry, but I'm stealing this because it annoys me so much and it gets duplicated because it confuses people. I ended up adding some more bits because otherwise the display wasn't getting updated at the right times, because the transition takes a bit to finish, yada yada.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8385624 [details] [diff] [review]
make zoom controls display the right thing in Australis even in customize mode,

Review of attachment 8385624 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, thanks!

Comment 8

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/1cec9e21e02b

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/1cec9e21e02b

Comment 10

[:Gijs (he/him)]

Comment on attachment 8385624 [details] [diff] [review]
make zoom controls display the right thing in Australis even in customize mode,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined: zoom reset button has confusing label (and has a label which can change) when in customize mode
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 11

[Mike Conley (:mconley) (:⚙️)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/38733824a12a