{"action":"created","comment":{"url":"https://api.github.com/repos/mozilla/universal-search/pulls/comments/55408821","id":55408821,"diff_hunk":"@@ -9,36 +9,143 @@ function HighlightManager(opts) {\n   this.events = opts.events;\n   // this is a recommendation view, because the highlight\n   // class manages passing focus between it and the list of results\n-  this.recommendationView = opts.recommendationView;\n+  this.recommendation = opts.recommendation;\n+  this.stealHighlightTimeout = null;\n \n   this.adjustHighlight = this.adjustHighlight.bind(this);\n+  this.stealHighlight = this.stealHighlight.bind(this);\n+  this.mutationHandler = this.mutationHandler.bind(this);\n+  this.initMutationObserver = this.initMutationObserver.bind(this);\n }\n \n HighlightManager.prototype = {\n   init: function() {\n     this.events.subscribe('navigational-key', this.adjustHighlight);\n-    this.events.subscribe('recommendation-shown', this.adjustHighlight);\n+    // It seems that the recommendation is rarely in the XUL DOM when this init\n+    // function runs, so listen for the recommendation to be found in the DOM,\n+    // then try a second time to attach the MutationObserver to the element.\n+    this.events.subscribe('recommendation-created', this.initMutationObserver);\n+    this.events.subscribe('recommendation-shown', this.stealHighlight);\n \n     this.popup = this.win.document.getElementById('PopupAutoCompleteRichResult');\n \n-    // TODO: maybe we can hook into the XBL mousemove handler and steal focus\n-    // that way instead of manually setting a mousemove listener\n-    this.popup.addEventListener('mousemove', this.adjustHighlight);\n+    this.initMutationObserver();\n+  },\n+  initMutationObserver: function() {\n+    // The existing code doesn't insert all the results at once; instead, rows\n+    // are inserted after repeated timeouts, to keep the urlbar responsive (see\n+    // _appendResultTimeout in autocomplete.xml). Each time a row is appended,\n+    // the existing code reapplies the highlight to an item in the results\n+    // list. So, as that happens, we want to continually steal the focus back.\n+    // The simplest way to do this, it turns out, is with a MutationObserver.\n+    // We listen for changes to the `url` attribute on the rows inside the\n+    // richlistbox. This ensures changes are detected, even when rows are\n+    // reused, rather than created (see _adjustAcItem in autocomplete.xml).\n+    //\n+    // TODO: Might want to throttle the callback if performance seems slow.\n+    this.resultsObserver = new this.win.MutationObserver(this.mutationHandler);\n+    const results = this.win.document.getAnonymousElementByAttribute(this.popup, 'anonid', 'richlistbox');\n+    const resultsObserverConfig = {\n+      childList: true,\n+      subtree: true,\n+      attributes: true,\n+      attributeFilter: ['url']\n+    };\n+    this.resultsObserver.observe(results, resultsObserverConfig);\n   },\n   destroy: function() {\n-    this.popup.removeEventListener('mousemove', this.adjustHighlight);\n-    this.events.unsubscribe('recommendation-shown', this.adjustHighlight);\n+    this.resultsObserver.disconnect();\n+    this.events.unsubscribe('recommendation-shown', this.stealHighlight);\n     this.events.unsubscribe('navigational-key', this.adjustHighlight);\n     delete this.popup;\n     delete this.recommendationView;\n     delete this.win;\n   },\n+  mutationHandler: function() {\n+    if (this.recommendation.el && !this.recommendation.el.collapsed) {\n+      this.stealHighlight();\n+    }\n+  },\n+  stealHighlight: function() {\n+    // When the recommendation has just been shown, if the default list item is\n+    // highlighted, steal the highlight and set it on the recommendation.\n+    //\n+    // The existing XBL code wraps each insertion in a setTimeout, so we can't\n+    // rely on setTimeout to steal the highlight: if 30 row rendering calls are\n+    // in the event loop ahead of this code, the blue highlight will very\n+    // noticeably flicker on the top result row. To avoid this, we steal the\n+    // highlight immediately, then use rAF to steal it just before the next\n+    // paint, then set *another* rAF inside the first rAF, preventing another\n+    // paint from showing a result in the list as highlighted. This seems a\n+    // little excessive, but actually the inner rAF seems to be the key to\n+    // truly minimizing the flickering as we steal the highlight.","path":"lib/ui/highlight-manager.js","position":80,"original_position":80,"commit_id":"1d86a57cafc6aa98e720d071e3166f1e513139fe","original_commit_id":"1d86a57cafc6aa98e720d071e3166f1e513139fe","user":{"login":"6a68","id":96396,"avatar_url":"https://avatars.githubusercontent.com/u/96396?v=3","gravatar_id":"","url":"https://api.github.com/users/6a68","html_url":"https://github.com/6a68","followers_url":"https://api.github.com/users/6a68/followers","following_url":"https://api.github.com/users/6a68/following{/other_user}","gists_url":"https://api.github.com/users/6a68/gists{/gist_id}","starred_url":"https://api.github.com/users/6a68/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/6a68/subscriptions","organizations_url":"https://api.github.com/users/6a68/orgs","repos_url":"https://api.github.com/users/6a68/repos","events_url":"https://api.github.com/users/6a68/events{/privacy}","received_events_url":"https://api.github.com/users/6a68/received_events","type":"User","site_admin":false},"body":"It is the height of madness, that's for sure. I guess the time when the setTimeouts will be executed is unpredictable, so if we spray the event loop with enough focus stealing DOM changes, we wind up doing pretty well on average?","created_at":"2016-03-08T19:01:32Z","updated_at":"2016-03-08T19:01:32Z","html_url":"https://github.com/mozilla/universal-search/pull/73#discussion_r55408821","pull_request_url":"https://api.github.com/repos/mozilla/universal-search/pulls/73","_links":{"self":{"href":"https://api.github.com/repos/mozilla/universal-search/pulls/comments/55408821"},"html":{"href":"https://github.com/mozilla/universal-search/pull/73#discussion_r55408821"},"pull_request":{"href":"https://api.github.com/repos/mozilla/universal-search/pulls/73"}}},"pull_request":{"url":"https://api.github.com/repos/mozilla/universal-search/pulls/73","id":62028108,"html_url":"https://github.com/mozilla/universal-search/pull/73","diff_url":"https://github.com/mozilla/universal-search/pull/73.diff","patch_url":"https://github.com/mozilla/universal-search/pull/73.patch","issue_url":"https://api.github.com/repos/mozilla/universal-search/issues/73","number":73,"state":"open","locked":false,"title":"WIP key focus handling","user":{"login":"6a68","id":96396,"avatar_url":"https://avatars.githubusercontent.com/u/96396?v=3","gravatar_id":"","url":"https://api.github.com/users/6a68","html_url":"https://github.com/6a68","followers_url":"https://api.github.com/users/6a68/followers","following_url":"https://api.github.com/users/6a68/following{/other_user}","gists_url":"https://api.github.com/users/6a68/gists{/gist_id}","starred_url":"https://api.github.com/users/6a68/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/6a68/subscriptions","organizations_url":"https://api.github.com/users/6a68/orgs","repos_url":"https://api.github.com/users/6a68/repos","events_url":"https://api.github.com/users/6a68/events{/privacy}","received_events_url":"https://api.github.com/users/6a68/received_events","type":"User","site_admin":false},"body":"@chuckharmston R? Have a little more cleanup to do, and the comments aren't all quite correct, but the focus management works reliably, with very occasional blue flickering in the top result row.\r\n    \r\nMost of the complexity here is due to the fact that the underlying autocomplete.xml code doesn't insert 30 results at once; instead, it sets 5 timeouts that insert 6 rows each. (The intention, according to a comment in the code, is to keep the urlbar responsive by batching updates.) The problem is, if we set a timeout to steal the focus, our timeout winds up behind the others on the stack, each of which causes tons of DOM updates, so that there's a long lag between the blue highlight being applied to the top result item, and our code unhighlighting that item. \r\n\r\nTo hack around this, we (1) use a MutationObserver to detect when the rows are changing or being inserted, and (2) in response to those mutation events, we steal focus three times: synchronously, on the next frame using `requestAnimationFrame`, and two frames ahead, via a nested `requestAnimationFrame` call.\r\n\r\n(This branch seems amazingly performant if you revert the dummy data commit. I'm kinda tempted to revert that in master, might open a PR to discuss tomorrow.)\r\n\r\nThere are some smaller changes, some of which need comments, but don't yet have them. I'll add comments before this lands; please call out anything that seems unclear. The squashed commit messages below describe the smaller changes:\r\n\r\n\r\n- Don't try to show/hide if the recommendation el doesn't yet exist\r\n- Separately handle grabbing the highlight when the rec is shown\r\n- Correct object graph initialization order\r\n- Track both the before and after popup hiding events\r\n- We may sometimes need to prevent the popup from hiding, by canceling\r\n\tthe 'popuphiding' XUL event (our 'before-popup-hide' event). In such\r\n\tcases, we don't want to hide the recommendation unless the popup is\r\n\treally hidden. XUL exposes the 'popuphidden' event, which we can hook\r\n\tinto as an 'after-popup-hide' event.\r\n- Remove mouse handlers from highlight manager for now, too much complexity\r\n- Avoid referring to gURLBar; we already have 'this.popup'\r\n- Always handle nav keys, now that we are replacing the XBL focus logic completely\r\n- send the recommendation-shown event after a turn, so it's in the dom\r\n- use mutation observer to steal focus over and over as rows are\r\n\tinserted into the popup\r\n- use 2 requestAnimationFrames to minimize the amount of blue highlight\r\n\tflickering on the zeroth result list item as results stream in.\r\n- take an initial swing at the highlight style","created_at":"2016-03-08T05:54:49Z","updated_at":"2016-03-08T19:01:32Z","closed_at":null,"merged_at":null,"merge_commit_sha":"9cd270d389291a07bee477ad59cb4235ab5c27c8","assignee":{"login":"chuckharmston","id":23885,"avatar_url":"https://avatars.githubusercontent.com/u/23885?v=3","gravatar_id":"","url":"https://api.github.com/users/chuckharmston","html_url":"https://github.com/chuckharmston","followers_url":"https://api.github.com/users/chuckharmston/followers","following_url":"https://api.github.com/users/chuckharmston/following{/other_user}","gists_url":"https://api.github.com/users/chuckharmston/gists{/gist_id}","starred_url":"https://api.github.com/users/chuckharmston/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chuckharmston/subscriptions","organizations_url":"https://api.github.com/users/chuckharmston/orgs","repos_url":"https://api.github.com/users/chuckharmston/repos","events_url":"https://api.github.com/users/chuckharmston/events{/privacy}","received_events_url":"https://api.github.com/users/chuckharmston/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/mozilla/universal-search/pulls/73/commits","review_comments_url":"https://api.github.com/repos/mozilla/universal-search/pulls/73/comments","review_comment_url":"https://api.github.com/repos/mozilla/universal-search/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mozilla/universal-search/issues/73/comments","statuses_url":"https://api.github.com/repos/mozilla/universal-search/statuses/1d86a57cafc6aa98e720d071e3166f1e513139fe","head":{"label":"6a68:WIP-38-key-focus-handling","ref":"WIP-38-key-focus-handling","sha":"1d86a57cafc6aa98e720d071e3166f1e513139fe","user":{"login":"6a68","id":96396,"avatar_url":"https://avatars.githubusercontent.com/u/96396?v=3","gravatar_id":"","url":"https://api.github.com/users/6a68","html_url":"https://github.com/6a68","followers_url":"https://api.github.com/users/6a68/followers","following_url":"https://api.github.com/users/6a68/following{/other_user}","gists_url":"https://api.github.com/users/6a68/gists{/gist_id}","starred_url":"https://api.github.com/users/6a68/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/6a68/subscriptions","organizations_url":"https://api.github.com/users/6a68/orgs","repos_url":"https://api.github.com/users/6a68/repos","events_url":"https://api.github.com/users/6a68/events{/privacy}","received_events_url":"https://api.github.com/users/6a68/received_events","type":"User","site_admin":false},"repo":{"id":51469648,"name":"universal-search","full_name":"6a68/universal-search","owner":{"login":"6a68","id":96396,"avatar_url":"https://avatars.githubusercontent.com/u/96396?v=3","gravatar_id":"","url":"https://api.github.com/users/6a68","html_url":"https://github.com/6a68","followers_url":"https://api.github.com/users/6a68/followers","following_url":"https://api.github.com/users/6a68/following{/other_user}","gists_url":"https://api.github.com/users/6a68/gists{/gist_id}","starred_url":"https://api.github.com/users/6a68/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/6a68/subscriptions","organizations_url":"https://api.github.com/users/6a68/orgs","repos_url":"https://api.github.com/users/6a68/repos","events_url":"https://api.github.com/users/6a68/events{/privacy}","received_events_url":"https://api.github.com/users/6a68/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/6a68/universal-search","description":"","fork":true,"url":"https://api.github.com/repos/6a68/universal-search","forks_url":"https://api.github.com/repos/6a68/universal-search/forks","keys_url":"https://api.github.com/repos/6a68/universal-search/keys{/key_id}","collaborators_url":"https://api.github.com/repos/6a68/universal-search/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/6a68/universal-search/teams","hooks_url":"https://api.github.com/repos/6a68/universal-search/hooks","issue_events_url":"https://api.github.com/repos/6a68/universal-search/issues/events{/number}","events_url":"https://api.github.com/repos/6a68/universal-search/events","assignees_url":"https://api.github.com/repos/6a68/universal-search/assignees{/user}","branches_url":"https://api.github.com/repos/6a68/universal-search/branches{/branch}","tags_url":"https://api.github.com/repos/6a68/universal-search/tags","blobs_url":"https://api.github.com/repos/6a68/universal-search/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/6a68/universal-search/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/6a68/universal-search/git/refs{/sha}","trees_url":"https://api.github.com/repos/6a68/universal-search/git/trees{/sha}","statuses_url":"https://api.github.com/repos/6a68/universal-search/statuses/{sha}","languages_url":"https://api.github.com/repos/6a68/universal-search/languages","stargazers_url":"https://api.github.com/repos/6a68/universal-search/stargazers","contributors_url":"https://api.github.com/repos/6a68/universal-search/contributors","subscribers_url":"https://api.github.com/repos/6a68/universal-search/subscribers","subscription_url":"https://api.github.com/repos/6a68/universal-search/subscription","commits_url":"https://api.github.com/repos/6a68/universal-search/commits{/sha}","git_commits_url":"https://api.github.com/repos/6a68/universal-search/git/commits{/sha}","comments_url":"https://api.github.com/repos/6a68/universal-search/comments{/number}","issue_comment_url":"https://api.github.com/repos/6a68/universal-search/issues/comments{/number}","contents_url":"https://api.github.com/repos/6a68/universal-search/contents/{+path}","compare_url":"https://api.github.com/repos/6a68/universal-search/compare/{base}...{head}","merges_url":"https://api.github.com/repos/6a68/universal-search/merges","archive_url":"https://api.github.com/repos/6a68/universal-search/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/6a68/universal-search/downloads","issues_url":"https://api.github.com/repos/6a68/universal-search/issues{/number}","pulls_url":"https://api.github.com/repos/6a68/universal-search/pulls{/number}","milestones_url":"https://api.github.com/repos/6a68/universal-search/milestones{/number}","notifications_url":"https://api.github.com/repos/6a68/universal-search/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/6a68/universal-search/labels{/name}","releases_url":"https://api.github.com/repos/6a68/universal-search/releases{/id}","deployments_url":"https://api.github.com/repos/6a68/universal-search/deployments","created_at":"2016-02-10T20:25:47Z","updated_at":"2016-02-02T18:16:02Z","pushed_at":"2016-03-08T05:48:41Z","git_url":"git://github.com/6a68/universal-search.git","ssh_url":"git@github.com:6a68/universal-search.git","clone_url":"https://github.com/6a68/universal-search.git","svn_url":"https://github.com/6a68/universal-search","homepage":null,"size":105,"stargazers_count":0,"watchers_count":0,"language":null,"has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"mozilla:master","ref":"master","sha":"6f7e852320b9d2f7da973e492543de18c1166cc4","user":{"login":"mozilla","id":131524,"avatar_url":"https://avatars.githubusercontent.com/u/131524?v=3","gravatar_id":"","url":"https://api.github.com/users/mozilla","html_url":"https://github.com/mozilla","followers_url":"https://api.github.com/users/mozilla/followers","following_url":"https://api.github.com/users/mozilla/following{/other_user}","gists_url":"https://api.github.com/users/mozilla/gists{/gist_id}","starred_url":"https://api.github.com/users/mozilla/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozilla/subscriptions","organizations_url":"https://api.github.com/users/mozilla/orgs","repos_url":"https://api.github.com/users/mozilla/repos","events_url":"https://api.github.com/users/mozilla/events{/privacy}","received_events_url":"https://api.github.com/users/mozilla/received_events","type":"Organization","site_admin":false},"repo":{"id":50941242,"name":"universal-search","full_name":"mozilla/universal-search","owner":{"login":"mozilla","id":131524,"avatar_url":"https://avatars.githubusercontent.com/u/131524?v=3","gravatar_id":"","url":"https://api.github.com/users/mozilla","html_url":"https://github.com/mozilla","followers_url":"https://api.github.com/users/mozilla/followers","following_url":"https://api.github.com/users/mozilla/following{/other_user}","gists_url":"https://api.github.com/users/mozilla/gists{/gist_id}","starred_url":"https://api.github.com/users/mozilla/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozilla/subscriptions","organizations_url":"https://api.github.com/users/mozilla/orgs","repos_url":"https://api.github.com/users/mozilla/repos","events_url":"https://api.github.com/users/mozilla/events{/privacy}","received_events_url":"https://api.github.com/users/mozilla/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mozilla/universal-search","description":"Test Pilot add-on that inserts server recommendations into the Awesome Bar","fork":false,"url":"https://api.github.com/repos/mozilla/universal-search","forks_url":"https://api.github.com/repos/mozilla/universal-search/forks","keys_url":"https://api.github.com/repos/mozilla/universal-search/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mozilla/universal-search/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mozilla/universal-search/teams","hooks_url":"https://api.github.com/repos/mozilla/universal-search/hooks","issue_events_url":"https://api.github.com/repos/mozilla/universal-search/issues/events{/number}","events_url":"https://api.github.com/repos/mozilla/universal-search/events","assignees_url":"https://api.github.com/repos/mozilla/universal-search/assignees{/user}","branches_url":"https://api.github.com/repos/mozilla/universal-search/branches{/branch}","tags_url":"https://api.github.com/repos/mozilla/universal-search/tags","blobs_url":"https://api.github.com/repos/mozilla/universal-search/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mozilla/universal-search/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mozilla/universal-search/git/refs{/sha}","trees_url":"https://api.github.com/repos/mozilla/universal-search/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mozilla/universal-search/statuses/{sha}","languages_url":"https://api.github.com/repos/mozilla/universal-search/languages","stargazers_url":"https://api.github.com/repos/mozilla/universal-search/stargazers","contributors_url":"https://api.github.com/repos/mozilla/universal-search/contributors","subscribers_url":"https://api.github.com/repos/mozilla/universal-search/subscribers","subscription_url":"https://api.github.com/repos/mozilla/universal-search/subscription","commits_url":"https://api.github.com/repos/mozilla/universal-search/commits{/sha}","git_commits_url":"https://api.github.com/repos/mozilla/universal-search/git/commits{/sha}","comments_url":"https://api.github.com/repos/mozilla/universal-search/comments{/number}","issue_comment_url":"https://api.github.com/repos/mozilla/universal-search/issues/comments{/number}","contents_url":"https://api.github.com/repos/mozilla/universal-search/contents/{+path}","compare_url":"https://api.github.com/repos/mozilla/universal-search/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mozilla/universal-search/merges","archive_url":"https://api.github.com/repos/mozilla/universal-search/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mozilla/universal-search/downloads","issues_url":"https://api.github.com/repos/mozilla/universal-search/issues{/number}","pulls_url":"https://api.github.com/repos/mozilla/universal-search/pulls{/number}","milestones_url":"https://api.github.com/repos/mozilla/universal-search/milestones{/number}","notifications_url":"https://api.github.com/repos/mozilla/universal-search/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mozilla/universal-search/labels{/name}","releases_url":"https://api.github.com/repos/mozilla/universal-search/releases{/id}","deployments_url":"https://api.github.com/repos/mozilla/universal-search/deployments","created_at":"2016-02-02T18:16:02Z","updated_at":"2016-03-02T18:00:39Z","pushed_at":"2016-03-08T18:17:59Z","git_url":"git://github.com/mozilla/universal-search.git","ssh_url":"git@github.com:mozilla/universal-search.git","clone_url":"https://github.com/mozilla/universal-search.git","svn_url":"https://github.com/mozilla/universal-search","homepage":"https://wiki.mozilla.org/Firefox/Universal_Search","size":68,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":4,"mirror_url":null,"open_issues_count":22,"forks":4,"open_issues":22,"watchers":0,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mozilla/universal-search/pulls/73"},"html":{"href":"https://github.com/mozilla/universal-search/pull/73"},"issue":{"href":"https://api.github.com/repos/mozilla/universal-search/issues/73"},"comments":{"href":"https://api.github.com/repos/mozilla/universal-search/issues/73/comments"},"review_comments":{"href":"https://api.github.com/repos/mozilla/universal-search/pulls/73/comments"},"review_comment":{"href":"https://api.github.com/repos/mozilla/universal-search/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mozilla/universal-search/pulls/73/commits"},"statuses":{"href":"https://api.github.com/repos/mozilla/universal-search/statuses/1d86a57cafc6aa98e720d071e3166f1e513139fe"}}}}