add extension chrome.windows.onFocusChanged #180
Conversation
| @@ -46,6 +47,57 @@ var binding = { | |||
| ipc.send('chrome-windows-update', responseId, windowId, updateInfo) | |||
| }, | |||
|
|
|||
| get: function (windowId, getInfo, cb) { | |||
There was a problem hiding this comment.
please use binding.registerCustomHook(function(bindingsAPI, extensionId) for all of these. See tab_bindings.js for examples
| @@ -1,6 +1,7 @@ | |||
| var ipc = require('ipc_utils') | |||
|
|
|||
| var id = 1; | |||
| var focusId = 1; | |||
There was a problem hiding this comment.
listeners don't have message ids because they are sent to all listeners
|
|
||
| }, | ||
|
|
||
| Filters: ['app', 'normal', 'panel', 'popup'] |
There was a problem hiding this comment.
if we don't handle these filters we should warn if they are used
There was a problem hiding this comment.
This was one of the issues I wanted to discuss. As far as I can tell, the Chrome API is in error here: it looks like they copy-pasted the text from onRemoved without updating it. While this text makes sense in the context of onRemoved, it doesn't shed any light on how onFocusChanged should work with filters.
At any rate, it isn't clear how window filters should work for onFocusChanged, if at all. The basic definitions I can think of to propose don't make sense if you follow them all the way through. Should this property even be present at all? Does someone know how it should work?
| cb(responseId); | ||
| }) | ||
|
|
||
| ipc.send('chrome-windows-focused-window-update', responseId) |
There was a problem hiding this comment.
this is only going to fire once. See tab_bindings.js for listener registration
| }) | ||
|
|
||
| apiFunctions.setHandleRequest('getCurrent', function () { | ||
| var cb, getInfo |
There was a problem hiding this comment.
setHandleRequest is supposed to normalize these so you always get two arguments even if the user only supplies one. I think this code should still work ok, but the argument length condition is no longer necessary
| @@ -480,6 +481,35 @@ tabEvents.forEach((event_name) => { | |||
| }) | |||
| }) | |||
|
|
|||
| ipcMain.on('register-chrome-window-focus', function (evt, extensionId) { | |||
| addBackgroundPageEvent(extensionId, 'chrome-window-focus') | |||
There was a problem hiding this comment.
I'm going to merge, but please update indenting for 2 spaces
| @@ -79,8 +78,10 @@ var sendToBackgroundPages = function (extensionId, session, event) { | |||
| var args = [].slice.call(arguments, 2) | |||
| pages.forEach(function (backgroundPage) { | |||
| try { | |||
| // only send to background pages in the same browser context | |||
| if (backgroundPage.session.equal(session)) { | |||
| var sendToAll = (typeof (session) === 'string' && session === 'all') | |||
There was a problem hiding this comment.
this change makes sense from a code perspective, but sending to all is a very special case and I'm a little concerned that this will get used where it shouldn't be. Maybe we should at least add a comment here explaining?
|
++ |
Event API: https://developer.chrome.com/extensions/windows#event-onFocusChanged