869933 - The zoom control should show current zoom level

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, enhancement)

Description

[u428464]

The reset button of Australis zoom controls (zoom out/reset/zoom in) currently always shows 100%. It would be more useful to have it indicate the current zoom level (especially for casual users). I can say taking into account what I've read on forums that it is the expected behavior (FWIW Google Chrome does it like that).

Resetting zoom could still be the button's default action or it should become only an indicator. UX feedback needed on this.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Adjust zoom reset button label whilst zooming in/out

Comment 2

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

Comment on attachment 748789 [details] [diff] [review]
Adjust zoom reset button label whilst zooming in/out

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

This patch doesn't work for synthetic documents (standalone images and standalone videos). You can test it with http://video.webmfiles.org/big-buck-bunny_trailer.webm.

This is because we don't store the zoom setting for synthetic documents, and the code in browser-fullZoom.js short-circuits before notifying observers in the case of .mozSyntheticDocument.

::: browser/locales/en-US/chrome/browser/browser.properties
@@ +457,5 @@
>  slowStartup.disableNotificationButton.label = Don't Tell Me Again
>  slowStartup.disableNotificationButton.accesskey = A
> +
> +# LOCALIZATION NOTE(zoomReset.label): %S is the current zoom level.
> +zoomReset.label = %S%

This needs to have two percentage characters to show a percentage sign. See https://developer.mozilla.org/en-US/docs/XUL/Tutorial/Property_Files#Text_Formatting for more details.

Comment 3

[Mike de Boer [:mikedeboer]]

Attachment: Adjust zoom reset button label whilst zooming in/out

fire events early and properly escape '%'

Comment 4

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

Comment on attachment 748864 [details] [diff] [review]
Adjust zoom reset button label whilst zooming in/out

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

Everything looks good except we should investigate why we need all the .init() calls in browser-fullZoom.js. I sure hope not all of those are necessary :)

Comment 5

[Mike de Boer [:mikedeboer]]

Attachment: Adjust zoom reset button label whilst zooming in/out

Removed all the init() calls and introduced a less disruptive change: call `FullZoom.init()` BEFORE `Panel().init()` in browser.js.

Comment 6

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

Comment on attachment 749214 [details] [diff] [review]
Adjust zoom reset button label whilst zooming in/out

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

::: browser/components/customizableui/content/panelUI.inc.xul
@@ +29,5 @@
>                           noautoclose="true" label="&fullZoomReduceCmd.label;"
>                           command="cmd_fullZoomReduce"
>                           tooltiptext="&zoomOutButton.tooltip;"/>
>            <toolbarbutton id="PanelUI-zoomReset-btn" flex="1"
> +                         noautoclose="true" label=""

This label attribute can be removed here.

Comment 7

[Mike de Boer [:mikedeboer]]

Attachment: Adjust zoom reset button label whilst zooming in/out

removed label. Carrying over r=jaws

Comment 8

[:Gijs (he/him)]

Pushed: http://hg.mozilla.org/projects/ux/rev/e545d3da3300

Comment 9

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/e545d3da3300