1616090 - If the default zoom level is not 100%, about:blank tabs always show zoom indicator [100%] in the address bar

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P5)

Description

[Alice0775 White]

Reproducible: always:

Steps to reproduce:

1. Set Home or New tabs to blank
2. Set default zoom level other than 100%
3. Open Home or New tab page or about:blank

Actual Results:
Blank tabs always show zoom indicator [100%] in the address bar

Expected Results:
the zoom indicator should not display

Comment 1

[Morgan Reschenberg [:morgan]]

So looks like this is because we manually set zoom of about:blank tabs here: https://searchfox.org/mozilla-central/rev/5a10be606f2d76ef22f1f44565749490de991d35/browser/base/content/browser-fullZoom.js#245

:gijs, do you think it makes sense to do a getGlobalValue here and send the "real" default zoom, or should I modify this [below] code to hide the URL bar badge for about:blank pages? The latter seems better performance-wise, since the only reason we'd be getting the value is to appease the conditional for the .hidden attribute, not to do any real zooming.

https://searchfox.org/mozilla-central/rev/5a10be606f2d76ef22f1f44565749490de991d35/browser/modules/ZoomUI.jsm#148

Comment 2

[:Gijs (out for now; he/him)]

(In reply to Morgan Reschenberg [:morgan] from comment #1)

So looks like this is because we manually set zoom of about:blank tabs here: https://searchfox.org/mozilla-central/rev/5a10be606f2d76ef22f1f44565749490de991d35/browser/base/content/browser-fullZoom.js#245

:gijs, do you think it makes sense to do a getGlobalValue here and send the "real" default zoom, or should I modify this [below] code to hide the URL bar badge for about:blank pages? The latter seems better performance-wise, since the only reason we'd be getting the value is to appease the conditional for the .hidden attribute, not to do any real zooming.

https://searchfox.org/mozilla-central/rev/5a10be606f2d76ef22f1f44565749490de991d35/browser/modules/ZoomUI.jsm#148

Right, I think for perf reasons it'd be better to hide the badge in the same case where we avoid calling getGlobalValue when setting the zoom (but only if the zoom is 1; if it's something else we should probably still compare to the global zoom, so that individual zoom for these pages does show the indicator?), assuming that's possible...

Comment 3

[Morgan Reschenberg [:morgan]]

(In reply to :Gijs (he/him) from comment #2)

Right, I think for perf reasons it'd be better to hide the badge in the same case where we avoid calling getGlobalValue when setting the zoom (but only if the zoom is 1; if it's something else we should probably still compare to the global zoom, so that individual zoom for these pages does show the indicator?), assuming that's possible...

when you say "in the same case where we avoid calling getGlobalValue when setting the zoom", do you mean in _applyPrefToZoom or in ZoomUI? It looks like the only way we avoid calling it in ZoomUI is if we early return before setting the .hidden attribute, so I'm not sure what ya mean.

The patch attached involves checking if the browser's current URI is about:blank, which is the same check the zoom code does. lemme know if you meant something different!

Comment 4

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1616090: Compare current URI to about:blank when setting zoom badge visibility. r?gijs

Comment 5

[:Gijs (out for now; he/him)]

I responded on phab. :-)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d55103f9020
Compare current URI to about:blank when setting zoom badge visibility. r=Gijs

Comment 7

[Razvan Maries]

Backed out for perma fails on browser_urlBar_zoom.js.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&searchStr=windows%2C10%2Cx64%2Cquantumrender%2Cshippable%2Copt%2Cmochitests%2Ctest-windows10-64-shippable-qr%2Fopt-mochitest-browser-chrome-e10s-2%2Cm%28bc2%29&revision=1d55103f9020fa24e38c083bc2cb0c8ab3a165a9&selectedJob=290875262

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=290875262&repo=autoland&lineNumber=5917

Backout: https://hg.mozilla.org/integration/autoland/rev/88d4fd05c8b5f8dde77b79577a059ffcbc4bfc56

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 9

[Morgan Reschenberg [:morgan]]

Fixed :)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53405c4200e1
Compare current URI to about:blank when setting zoom badge visibility. r=Gijs

Comment 11

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/53405c4200e1