Closed Bug 1454440 Opened 2 years ago Closed 2 years ago

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

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file)

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.
Summary: Look into using `win.focus` instead of `win.document.commandDispatcher.focusedWindow.focus()` in macWindowMenu.js → Look into using `win.focus()` instead of `win.document.commandDispatcher.focusedWindow.focus()` in macWindowMenu.js
This line was added in Bug 1425363 when we stopped using rdf templates for the window menu.
Depends on: 1425363
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
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. :-)
Attachment #8968268 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32220a5b1ba8
Focus the window directly instead of using commandDispatcher in the mac Window menu;r=Gijs
https://hg.mozilla.org/mozilla-central/rev/32220a5b1ba8
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61

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.)

You need to log in before you can comment on or make changes to this bug.