1378784 - Zoom Indicator Missing When Using Zoom Text Only

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Aaron Smith]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170628075643

Steps to reproduce:

Use the Zoom Text Only feature, and increase or decrease the zoom level.


Actual results:

The zoom level indicator does not display.


Expected results:

The zoom level indicator should display as it does when Zoom Text Only is disabled. This is a regression, as it used to work in the previous version.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2ef7b06c606b0334e2a9634ae02b98339ffa7aeb&tochange=e263ffa2eb294aaca7a1935e23de522f71e2ed01


Regressed by: 
e263ffa2eb29	Jared Wein — Bug 1345375 - Use the FullZoomChange event instead of browser-fullZoom observers since FullZoomChange works on MediaDocuments. r=dao

Comment 3

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

Attachment: Bug 1378784 - Listen for TextZoomChange in addition to the FullZoomChange events to update the zoom UI when the zoom changes.

We were already listening for FullZoomChange. TextZoomChange is very similar, but is only triggered when the user has chosen 'Zoom Text Only' from the View > Zoom menu.

Review commit: https://reviewboard.mozilla.org/r/158556/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158556/

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 8887669 [details]
Bug 1378784 - Listen for TextZoomChange in addition to the FullZoomChange events to update the zoom UI when the zoom changes.

https://reviewboard.mozilla.org/r/158556/#review164038

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6371d6ad918f
Listen for TextZoomChange in addition to the FullZoomChange events to update the zoom UI when the zoom changes. r=dao

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6371d6ad918f

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Jared, did you want to nominate this for Beta or let it ride the 56 train?

Comment 8

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

Comment on attachment 8887669 [details]
Bug 1378784 - Listen for TextZoomChange in addition to the FullZoomChange events to update the zoom UI when the zoom changes.

Approval Request Comment
[Feature/Bug causing the regression]: regressed by bug 1345375 in Firefox 55
[User impact if declined]: users who use Text Zoom won't see the zoom indicator in the URL bar
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: Change the zoom to Text Only (On Windows, Alt+V -> Zoom -> Zoom Text Only), then zoom in and out of the page (Ctrl+ and Ctrl-, respectively). The zoom indicator should appear in the location bar if the zoom value is anything other than 100%.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no, just adds another listener for the TextZoomChange, we already were listening for FullZoomChange.
[Why is the change risky/not risky?]: see previous answer
[String changes made/needed]: none

Comment 9

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

Minor correction, the regressing bug was initially landed on 55 but was later uplifted to 54 so we have shipped this regression for one release already.

Comment 10

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

update, if this gets beta approval, then we should uplift the patches in bug 1383662 also.

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 8887669 [details]
Bug 1378784 - Listen for TextZoomChange in addition to the FullZoomChange events to update the zoom UI when the zoom changes.

I think I'll call this wontfix for 55, since we've shipped this bug already in 54, it doesn't seem critical, and it's late in the cycle with a single build left before RC.

Comment 12

[Alice0775 White]

no automated test?

Comment 13

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

(In reply to Alice0775 White from comment #12)
> no automated test?

I filed bug 1384522 to add an automated test.

Comment 14

[Saddam Hossain]

I have reproduced this Bug on Nightly 56.0a1 (2017-07-06) on Windows 10, 64 bit!

The bug's fix is now verified on latest  Nightly 56.0a1

Build ID    :	20170729100254
User Agent  : 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

Comment 15

[suba4598]

bug has been fixed and resolved

Comment 16

[Bogdan Maris, Desktop Test Engineering [OOO until Dec 16th]]

I managed to reproduce this bug on an old version of Nightly from 2017-06-15 on Windows 10x64, macOS 10.12 and Ubuntu 16.04 x64.
I retested everything using the same platforms on beta 56.0b11 and latest Nightly 57.0a1 and the bug seems fixed.