Simplify logic for locating the scrollRoot element

Currently the app only ever has one `<thread-list>` instance, so
we can simplify the logic that gets the scroll root to just query for a
hard-coded selector.

src/sidebar/components/test/thread-list-test.js

@@ -82,7 +82,7 @@ describe('threadList', function () {
     // Create a scrollable container for the `<thread-list>` so that scrolling
     // can be tested.
     var parentEl = document.createElement('div');
-    parentEl.classList.add('js-scrollable');
+    parentEl.classList.add('js-thread-list-scroll-root');
     parentEl.style.overflow = 'scroll';
     parentEl.style.height = '100px';
 
@@ -213,15 +213,9 @@ describe('threadList', function () {
       sinon.match(annotFixtures.annotation));
   });
 
-  // See https://github.com/hypothesis/client/issues/341
-  it('finds correct scroll root if `window.getComputedStyle` fails', function () {
-    sinon.stub(window, 'getComputedStyle').returns(null);
-    var element = createThreadList();
-    element.scope.$digest();
-
+  it('uses the correct scroll root', function () {
+    createThreadList();
     var scrollRoot = fakeVirtualThread.options.scrollRoot;
-    assert.isTrue(scrollRoot.classList.contains('js-scrollable'));
-
-    window.getComputedStyle.restore();
+    assert.isTrue(scrollRoot.classList.contains('js-thread-list-scroll-root'));
   });
 });

src/sidebar/components/thread-list.js

@@ -43,39 +43,6 @@ var virtualThreadOptions = {
   },
 };
 
-/**
- * Return true if the given element is scrollable.
- */
-function isScrollable(el) {
-  // Check the computed style to see if this element is marked as scrollable.
-  var computedStyle = window.getComputedStyle(el);
-  if (computedStyle !== null && computedStyle.overflowY === 'scroll') {
-    return true;
-  }
-  // In Firefox, `getComputedStyle` may return `null` if `window` is inside a
-  // non-rendered (`display:none`) iframe. In this case, fall back to requiring
-  // the developer to annotate the element with a special class.
-  // See https://bugzilla.mozilla.org/show_bug.cgi?id=548397 and
-  // https://github.com/w3c/csswg-drafts/issues/572
-  return el.classList.contains('js-scrollable');
-}
-
-/**
- * Return the closest ancestor of `el` which is scrollable.
- *
- * @param {Element} el
- */
-function closestScrollableAncestor(el) {
-  var parentEl = el;
-  while (parentEl !== document.body) {
-    if (isScrollable(parentEl)) {
-      return parentEl;
-    }
-    parentEl = parentEl.parentElement;
-  }
-  return parentEl;
-}
-
 // @ngInject
 function ThreadListController($element, $scope, VirtualThreadList) {
   // `visibleThreads` keeps track of the subset of all threads matching the
@@ -86,7 +53,12 @@ function ThreadListController($element, $scope, VirtualThreadList) {
 
   // `scrollRoot` is the `Element` to scroll when scrolling a given thread into
   // view.
-  this.scrollRoot = closestScrollableAncestor($element[0]);
+  //
+  // For now there is only one `<thread-list>` instance in the whole
+  // application so we simply require the scroll root to be annotated with a
+  // specific class. A more generic mechanism was removed due to issues in
+  // Firefox. See https://github.com/hypothesis/client/issues/341
+  this.scrollRoot = document.querySelector('.js-thread-list-scroll-root');
 
   var options = Object.assign({
     scrollRoot: this.scrollRoot,

src/sidebar/templates/hypothesis_app.html

-<div class="app-content-wrapper js-scrollable" h-branding="appBackgroundColor">
+<div class="app-content-wrapper js-thread-list-scroll-root" h-branding="appBackgroundColor">
   <top-bar
     auth="vm.auth"
     on-login="vm.login()"