FEATURE: Stricter rules for user presence

FEATURE: Stricter rules for user presence

Previously we would consider a user “present” and “last seen” if the browser window was visible.

This has many edge cases, you could be considered present and around for days just by having a window open and no screensaver on.

Instead we now also check that you either clicked, transitioned around app or scrolled the page in the last minute in combination with window visibility

This will lead to more reliable notifications via email and reduce load of message bus for cases where a user walks away from the terminal

diff --git a/Gemfile.lock b/Gemfile.lock
index 471bb0f..08056d6 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -182,7 +182,7 @@ GEM
       mini_mime (>= 0.1.1)
     maxminddb (0.1.22)
     memory_profiler (0.9.14)
-    message_bus (2.2.3)
+    message_bus (2.2.4)
       rack (>= 1.1.3)
     metaclass (0.0.4)
     method_source (0.9.2)
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js
index 35cc29a..9addddf 100644
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -11,7 +11,7 @@
 // Stuff we need to load first
 //= require ./discourse/lib/to-markdown
 //= require ./discourse/lib/utilities
-//= require ./discourse/lib/page-visible
+//= require ./discourse/lib/user-presence
 //= require ./discourse/lib/logout
 //= require ./discourse/mixins/singleton
 //= require ./discourse/models/rest
diff --git a/app/assets/javascripts/discourse/initializers/message-bus.js b/app/assets/javascripts/discourse/initializers/message-bus.js
index 960aaaa..61905e4 100644
--- a/app/assets/javascripts/discourse/initializers/message-bus.js
+++ b/app/assets/javascripts/discourse/initializers/message-bus.js
@@ -1,5 +1,5 @@
 // Initialize the message bus to receive messages.
-import pageVisible from "discourse/lib/page-visible";
+import userPresent from "discourse/lib/user-presence";
 import { handleLogoff } from "discourse/lib/ajax";
 
 function ajax(opts) {
@@ -31,6 +31,7 @@ export default {
       siteSettings = container.lookup("site-settings:main");
 
     messageBus.alwaysLongPoll = Discourse.Environment === "development";
+    messageBus.shouldLongPollCallback = userPresent;
 
     // we do not want to start anything till document is complete
     messageBus.stop();
@@ -65,16 +66,16 @@ export default {
         opts.headers["X-Shared-Session-Key"] = $(
           "meta[name=shared_session_key]"
         ).attr("content");
-        if (pageVisible()) {
-          opts.headers["Discourse-Visible"] = "true";
+        if (userPresent()) {
+          opts.headers["Discourse-Present"] = "true";
         }
         return ajax(opts);
       };
     } else {
       messageBus.ajax = function(opts) {
         opts.headers = opts.headers || {};
-        if (pageVisible()) {
-          opts.headers["Discourse-Visible"] = "true";
+        if (userPresent()) {
+          opts.headers["Discourse-Present"] = "true";
         }
         return ajax(opts);
       };
diff --git a/app/assets/javascripts/discourse/lib/ajax.js b/app/assets/javascripts/discourse/lib/ajax.js
index 54de18d..37052a4 100644
--- a/app/assets/javascripts/discourse/lib/ajax.js
+++ b/app/assets/javascripts/discourse/lib/ajax.js
@@ -1,5 +1,5 @@
 import { run } from "@ember/runloop";
-import pageVisible from "discourse/lib/page-visible";
+import userPresent from "discourse/lib/user-presence";
 import logout from "discourse/lib/logout";
 import Session from "discourse/models/session";
 import { Promise } from "rsvp";
@@ -92,8 +92,8 @@ export function ajax() {
       args.headers["Discourse-Track-View"] = "true";
     }
 
-    if (pageVisible()) {
-      args.headers["Discourse-Visible"] = "true";
+    if (userPresent()) {
+      args.headers["Discourse-Present"] = "true";
     }
 
     args.success = (data, textStatus, xhr) => {
diff --git a/app/assets/javascripts/discourse/lib/user-presence.js b/app/assets/javascripts/discourse/lib/user-presence.js
new file mode 100644
index 0000000..67d3329
--- /dev/null
+++ b/app/assets/javascripts/discourse/lib/user-presence.js
@@ -0,0 +1,45 @@
+// for android we test webkit
+const hiddenProperty =
+  document.hidden !== undefined
+    ? "hidden"
+    : document.webkitHidden !== undefined
+    ? "webkitHidden"
+    : undefined;
+
+const MAX_UNSEEN_TIME = 60000;
+
+let seenUserTime = Date.now();
+
+export default function() {
+  const now = Date.now();
+
+  if (seenUserTime + MAX_UNSEEN_TIME < now) {
+    return false;
+  }
+
+  if (hiddenProperty !== undefined) {
+    return !document[hiddenProperty];
+  } else {
+    return document && document.hasFocus;
+  }
+}
+
+export function seenUser() {
+  seenUserTime = Date.now();
+}
+
+// We could piggieback on the Scroll mixin, but it is not applied
+// consistently to all pages
+//
+// We try to keep this as cheap as possible by performing absolute minimal
+// amount of work when the event handler is fired
+//
+// An alternative would be to use a timer that looks at the scroll position
+// however this will not work as message bus can issue page updates and scroll
+// page around when user is not present
+//
+// We avoid tracking mouse move which would be very expensive
+
+$(document).bind("touchmove.discourse-track-presence", seenUser);
+$(document).bind("click.discourse-track-presence", seenUser);
+$(window).bind("scroll.discourse-track-presence", seenUser);
diff --git a/app/assets/javascripts/discourse/routes/discourse.js b/app/assets/javascripts/discourse/routes/discourse.js
index 980e986..91be0b4 100644
--- a/app/assets/javascripts/discourse/routes/discourse.js
+++ b/app/assets/javascripts/discourse/routes/discourse.js
@@ -3,10 +3,15 @@ import Composer from "discourse/models/composer";
 import { getOwner } from "discourse-common/lib/get-owner";
 import Route from "@ember/routing/route";
 import deprecated from "discourse-common/lib/deprecated";
+import { seenUser } from "discourse/lib/user-presence";
 
 const DiscourseRoute = Route.extend({
   showFooter: false,
 
+  willTransition() {
+    seenUser();
+  },
+
   // Set to true to refresh a model without a transition if a query param
   // changes
   resfreshQueryWithoutTransition: false,
diff --git a/config/initializers/004-message_bus.rb b/config/initializers/004-message_bus.rb
index da12f03..924fe6d 100644
--- a/config/initializers/004-message_bus.rb
+++ b/config/initializers/004-message_bus.rb
@@ -30,7 +30,7 @@ def setup_message_bus_env(env)
     extra_headers = {
       "Access-Control-Allow-Origin" => Discourse.base_url_no_prefix,
       "Access-Control-Allow-Methods" => "GET, POST",
-      "Access-Control-Allow-Headers" => "X-SILENCE-LOGGER, X-Shared-Session-Key, Dont-Chunk, Discourse-Visible"
+      "Access-Control-Allow-Headers" => "X-SILENCE-LOGGER, X-Shared-Session-Key, Dont-Chunk, Discourse-Present"
     }
 
     user = nil
diff --git a/config/initializers/008-rack-cors.rb b/config/initializers/008-rack-cors.rb
index 5f1c6f4..cc5bb8c 100644
--- a/config/initializers/008-rack-cors.rb
+++ b/config/initializers/008-rack-cors.rb
@@ -39,7 +39,7 @@ class Discourse::Cors
       end
 
       headers['Access-Control-Allow-Origin'] = origin || cors_origins[0]
-      headers['Access-Control-Allow-Headers'] = 'Content-Type, Cache-Control, X-Requested-With, X-CSRF-Token, Discourse-Visible, User-Api-Key, User-Api-Client-Id, Authorization'
+      headers['Access-Control-Allow-Headers'] = 'Content-Type, Cache-Control, X-Requested-With, X-CSRF-Token, Discourse-Present, User-Api-Key, User-Api-Client-Id, Authorization'
       headers['Access-Control-Allow-Credentials'] = 'true'
       headers['Access-Control-Allow-Methods'] = 'POST, PUT, GET, OPTIONS, DELETE'
     end
diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb
index ca8ec4d..3e293dc 100644
--- a/lib/auth/default_current_user_provider.rb
+++ b/lib/auth/default_current_user_provider.rb
@@ -251,7 +251,7 @@ class Auth::DefaultCurrentUserProvider
     api = !!(@env[API_KEY_ENV]) || !!(@env[USER_API_KEY_ENV])
 
     if @request.xhr? || api
-      @env["HTTP_DISCOURSE_VISIBLE".freeze] == "true".freeze
+      @env["HTTP_DISCOURSE_PRESENT"] == "true"
     else
       true
     end
diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb
index 2fd165c..f0c6fbc 100644
--- a/spec/components/auth/default_current_user_provider_spec.rb
+++ b/spec/components/auth/default_current_user_provider_spec.rb

[... diff too long, it was truncated ...]

GitHub sha: 25f1f232

Scroll listeners should be explicitly set as passive: true, and the others as well I think.

1 Like

Not sure about this for other use cases, cause we want to update the UX (specifically timeline) when you scroll through a topic.

For this specific one it can be passive, though the odds of this blocking are pretty much 0.

Date.now() can run 7 million times a second on my browser

https://jsperf.com/gettime-vs-now-0/7

Given this is not supported in jQuery per:

And we have bind central throughout our code base, I think we should leave this battle to another day when @jjaffeux eventually gets around to replacing various binds.

The perf impact is just not there.

Yes I should dig into this someday. Also minor but ie11 doesnt support this afaik.

1 Like

I remember that some versions of Chrome for Android would tank scroll performance back to pre-Flinger levels if any non-passive listener was attached, but I think nowadays it won’t do that until it actually sees a cancellation attempt.