698873 - Disable View > Zoom > "Actual size" when the zoom level is already at default.

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, enhancement, P3)

Description

[jonathan chetwynd]

Opera and Safari indicate when the user cannot reset the zoom level,

Opera has a tick on 100%

Safari greys out the reset option in the toolbar.

Mozilla could also provide an indicator in the address bar

Comment 2

[jonathan chetwynd]

this is not a dupe, that bug request is in respect of showing the zoom level,
IE, Safari and Opera all show when the zoom level cannot be reset.

Comment 3

[jonathan chetwynd]

to be clear one fix would be
View Zoom Reset being greyed out when zoom is 100% and hence cannot be reset.

the bug 234309 does not specifically mention this.

Comment 4

[Rogério Gonçalves]

Attachment: Patch to this bug. Disable Zoom in, Zoom out and Reset controls when values are reached

Marcos Silvano and I worked together in this patch.

Comment 5

[Rogério Gonçalves]

Attachment: Video of result obtained.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 589456 [details] [diff] [review]
Patch to this bug. Disable Zoom in, Zoom out and Reset controls when values are reached

Thanks for picking this up, Rogério. This looks good, but there's one problem: this function is called onpopupshowing, and the command elements are used for actions that aren't related to the popup (for example, the shortcut keys). So you can't just set their state in the popupshowing handler and then leave it that way, because it will break the commands in other pages (if you don't open the menu first).

So rather than using goSetCommandEnabled to edit the command's disabled state, let's just update the menuitems themselves (using e.g. document.getElementById("key_fullZoomReset") with setAttribute/removeAttribute for "disabled").

Comment 7

[Rogério Gonçalves]

Gavin,
I've used the function document.getElementById('menu_zoomReset').setAttribute('disabled', true) and nothing happened.
The document.getElementById('menu_zoomReset').removeAttribute('disabled') produces no effects too. 
I've also tried these functions with the element "key_fullZoomReset" and nothing happens.
I've tried these methods before and get the same results. 
That's why I used the goSetCommandEnabled() function.

Am I missing something?

Comment 8

[Rogério Gonçalves]

Attachment: The second version patch to this bug with the suggested modifications by Gavin.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 596245 [details] [diff] [review]
The second version patch to this bug with the suggested modifications by Gavin.

We really can't have the poupshowing handler change the "disabled" state of the command elements, at least not without equivalent changes to ensure that the commands get re-enabled appropriately.

Setting the disabled attribute directly on the menu item might not work because of it's association with the <command> element. You can test this theory by removing the command="" attribute temporarily and seeing if my suggestion works then.

Comment 10

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

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 11

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

Attachment: Bug 698873 - Update Zoom commands when changing zoom level. r?NeilDeakin!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/768a911921b0
Update Zoom commands when changing zoom level. r=NeilDeakin,desktop-theme-reviewers

Comment 13

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing failures on browser_preferences_usage.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/e27f007bc0fdf42e1b2ca39a2797745adcefd561

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=e_pj4z6yTIqUVxWhvQIU1w.0&resultStatus=usercancel%2Cretry%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&revision=768a911921b06c07fa7187227d35f7a4e196e67e

Link to failure log: https://treeherder.mozilla.org/logviewer?job_id=380932755&repo=autoland&lineNumber=5908

Failure line:
TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_preferences_usage.js | browser.zoom.full should be accessed at most 25 times. - 41 <= 25 - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/browser_preferences_usage.js :: checkPrefGetters :: line 63

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ced61ad7cf8f
Update Zoom commands when changing zoom level. r=NeilDeakin,desktop-theme-reviewers

Comment 15

[Narcis Beleuzu [:NarcisB]]

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