1454440 - Look into using `win.focus()` instead of `win.document.commandDispatcher.focusedWindow.focus()` in macWindowMenu.js

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Brian Grinstead [:bgrins]]

https://searchfox.org/mozilla-central/rev/bd326a2a6b729dc62a5aee57354a97ceac4d1dc0/toolkit/content/macWindowMenu.js#24

Since document.commandDispatcher doesn't work in HTML documents, this line throws when attempting to load webconsole.html directly in the Browser Console window.

Comment 1

[Brian Grinstead [:bgrins]]

This line was added in Bug 1425363 when we stopped using rdf templates for the window menu.

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: Bug 1454440 - Focus the window directly instead of using commandDispatcher in the mac Window menu;

This fixes the Window menu with top-level HTML windows, which don't have `commandDispatcher`
defined on the document.

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

Comment 3

[Brian Grinstead [:bgrins]]

Awaiting try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e2f51711013f53ee644adf78431d76599ad4d3d

Comment 4

[:Gijs (he/him)]

Comment on attachment 8968268 [details]
Bug 1454440 - Focus the window directly instead of using commandDispatcher in the mac Window menu;

https://reviewboard.mozilla.org/r/236954/#review242826

Looks like Hyatt first landed this in CVS 15 or thereabouts years ago, before Firefox, https://hg.mozilla.org/experimental/mozilla-central-cvs/rev/393af2ac6e56 (notice the mixed spaces and tabs, and lack of review... ;-) ) . I haven't tested this, but I assume you have, and it LGTM. :-)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/32220a5b1ba8
Focus the window directly instead of using commandDispatcher in the mac Window menu;r=Gijs

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/32220a5b1ba8

Comment 7

[[e-mail]]

The reason the old code used the command dispatcher is that at the time it was written, calling window.focus would always cause the address bar to become focused, because it performed what is now nsIFocusManager::SetFocusedWindow, when what you really want is nsIFocusManager::SetActiveWindow. (I think window.focus does set the active window these days, but I'm not 100% sure.)