Update social media modules #54

akohlbre · 2021-04-10T03:47:04Z

Adds support for blocking to the events module (to allow for a module to wait on the result of a listener)
Updates socialMediaActivity to use the event model
Updates socialMediaLinkSharing to use the new interface to socialMediaActivity
Adds documentation in socialMediaActivity

Remaining tasks for these modules are in #17 and #23.

src/events.js

src/socialMediaActivity.js

jonathanmayer · 2021-04-10T03:55:06Z

src/socialMediaActivity.js

- * @param blocking - whether the listener should be blocking. Allows canceling the event.
+ * Filter generated events to only the listeners for the specific event type.
+ * @param {socialMediaActivityCallback} listener - The listener that may be notified.
+ * @param {Array} listenerArguments - The event about to be sent to the listener.


Array of what?

src/socialMediaActivity.js

jonathanmayer · 2021-04-10T03:57:56Z

src/socialMediaActivity.js

+/**
+ * @type {events.event<socialMediaActivityCallback, socialMediaActivityOptions}
+ */


See how events are annotated in other modules. We might eventually change the format, but it's the least-bad I could come up with for now.

jonathanmayer · 2021-04-10T03:59:49Z

src/socialMediaActivity.js

+export const onSocialMediaActivity = events.createEvent({
+    addListenerCallback: addListener,
+    notifyListenersCallback: notifyFilter
+});


A big-picture design question for socialMediaActivity and socialMediaLinkSharing: do we want one event for all the platforms, or one event per platform? An advantage of the former is we avoid some repetition. An advantage of the latter is library users might otherwise shoot themselves in the feet by mixing and matching platforms and activity types.

src/socialMediaActivity.js

jonathanmayer · 2021-04-10T04:04:48Z

src/socialMediaActivity.js

+
+/**
+ * Options when adding a social media activity event listener.
+ * @typedef {Object} socialMediaActivityOptions


I've been trying to establish a convention of lower camel case for callback types and upper camel case for other types (including @callback functions that aren't really callbacks).

src/socialMediaActivity.js

jonathanmayer · 2021-04-10T04:09:22Z

src/socialMediaActivity.js

 /**
+ * Determine the audience of a Facebook post or reshare.


Here and throughout, when parsing platform-specific values, please make sure to document what the source data is and what assumptions you're making in parsing.

jonathanmayer · 2021-04-10T04:11:16Z

src/socialMediaActivity.js

+ * This information is not included in the web request, nor is it attached to the generated post,
+ * so the only way to get it is by watching the user click the reshare button, which
+ * the content script does.
+ * @return {Promise} - The source of the last reshare ("person" or "page").


What type of Promise?

jonathanmayer · 2021-04-10T04:11:49Z

src/socialMediaActivity.js

+/**
+ * For a given subreddit name, return whether it is private or public.
+ * @param {string} subredditName - The name of the subreddit, without leading "r/".
+ * @return {Promise} - resolves to a string representing the subreddit's status, or "unknown".


What type of Promise?

…ce into socialmedia-events

standardize return values add support for facebook reshares with 'include original post' option add support for old reddit

akohlbre · 2021-06-21T03:18:31Z

This PR now contains all the code changes we're currently planning. I'll be pushing more documentation updates shortly, namely:

Longer explanatory comments about usage at the top of SMA and SMLS
Using the @namespace convention for documenting events and their associated object
Clearer documentation for which fields are relevant to which event types in SMA
More specific documentation about how we interact with platforms
More specific documentation for a few return types (e.g. anything currently just @returns {Object})

jonathanmayer · 2021-06-21T04:40:21Z

src/content-scripts/socialMediaActivity.facebook.content.js

@@ -23,6 +23,10 @@ import { facebookLinkShimRegExp, parseFacebookLinkShim, removeFacebookLinkDecora
    const trackedReshares = []
    let mostRecentReshare = null;

+    /**


Rather than having content scripts check a private window permission with extension storage (lines 13-16 above), we've migrated to sending the private window status and letting the background script filter events. See other modules for examples.

jonathanmayer · 2021-06-21T04:42:39Z

src/content-scripts/socialMediaActivity.facebook.content.js

@@ -23,6 +23,10 @@ import { facebookLinkShimRegExp, parseFacebookLinkShim, removeFacebookLinkDecora
    const trackedReshares = []
    let mostRecentReshare = null;

+    /**
+     * Note the source of a reshare when the user clicks the button, in case it's wanted later.
+     * @param {Object} clicked - the page element that we were listening for clicks on.


Is this a DOM Element? Could you give a more specific type?

jonathanmayer · 2021-06-21T04:43:10Z

src/content-scripts/socialMediaActivity.facebook.content.js

    const trackedReshares = []
    let mostRecentReshare = null;


Documentation!

jonathanmayer · 2021-06-21T04:44:42Z

src/content-scripts/socialMediaActivity.facebook.content.js

@@ -63,6 +70,7 @@ import { facebookLinkShimRegExp, parseFacebookLinkShim, removeFacebookLinkDecora
        }
    }

+    // periodically add listeners to newly-loaded posts.
    setInterval(() => reshareSourceTracking(), 3000);


Could just give reshareSourceTracking as the callback. Would also specify the interval as a constant and pause the timer when the page doesn't have attention.

jonathanmayer · 2021-06-21T04:45:13Z

src/content-scripts/socialMediaActivity.facebook.content.js

@@ -72,8 +80,8 @@ import { facebookLinkShimRegExp, parseFacebookLinkShim, removeFacebookLinkDecora
     * @param response -- an object to add found links to


jonathanmayer · 2021-06-21T06:17:01Z

src/socialMediaLinkSharing.js

    if (facebook) {
-        socialMediaActivity.registerFacebookActivityTracker(facebookLinks, ["post", "reshare"]);
+        socialMediaActivity.onFacebookActivity.addListener(facebookLinks, {
+            eventTypes: ['post', 'reshare']});
    }
    if (reddit) {
-        socialMediaActivity.registerRedditActivityTracker(redditLinks, ["post"]);
+        socialMediaActivity.onRedditActivity.addListener(redditLinks, {
+            eventTypes: ['post']});
    }
    if (twitter) {
-        socialMediaActivity.registerTwitterActivityTracker(twitterLinks, ["tweet", "retweet", "favorite"]);
+        socialMediaActivity.onTwitterActivity.addListener(twitterLinks, {
+            eventTypes: ['tweet', 'retweet', 'favorite']});
    }


Seems like some logic is needed in here for adding multiple listeners.

jonathanmayer · 2021-06-21T06:18:24Z

src/socialMediaLinkSharing.js

+/**
+ * Given events that were set up to be "tracked"-type notifications but do not match the current
+ *  listener's match patterns, turn them into one or more "untracked"-type notifications instead.
+ * @param {socialMediaShareDetails[]} trackedEvents - a set of non-matching share events.
+ * @returns {socialMediaShareDetails[]} - the tracked events stripped and condensed into untracked events.
+ * @private
+ */
+function trackedToUntracked(trackedEvents) {


Would just combine this with the prior function.

jonathanmayer · 2021-06-21T06:18:50Z

src/socialMediaLinkSharing.js

+            newEvents.push(newEvent);
+        }
+    }
+    return newEvents;
 }

 /**


Need to add logic for teardown after removing listeners.

jonathanmayer · 2021-06-21T06:19:19Z

src/socialMediaLinkSharing.js

 * @private
 */
 function isTwitterLink(url) {
-    const twitterLink = /twitter\.com\/[0-9|a-z|A-Z|_]*\/status\/([0-9]*)\//;
+    const twitterLink = /twitter\.com\/[0-9|a-z|A-Z|_]*\/status\/([0-9]*)/;


Would parse to a URL, check the hostname, then check the path.

jonathanmayer · 2021-06-21T06:33:58Z

src/socialMediaLinkSharing.js

 */
-const debugLog = debugging.getDebuggingLog("socialMediaLinkSharing");
+export const onShare = events.createEvent({


I think the boundary between socialMediaActivity and socialMediaLinkSharing should be a bit different from what it is. Currently, socialMediaActivity exposes functions for looking up tweets, posts, and subreddits, and socialMediaLinkSharing has to call those functions. It would be preferable if we could keep those functions internal to socialMediaActivity and only expose them if study developers eventually need them. The information that socialMediaLinkSharing has to obtain itself is also information that many callers to socialMediaActivity would want anyway. A possible approach would be a listener option for socialMediaActivity that indicates shared tweets and posts should be resolved, plus adding subreddit status to reddit events.

jonathanmayer · 2021-06-21T07:05:52Z

src/socialMediaActivity.js

+ * @property {number} eventTime - timestamp of user action.
+ * @property {string} tweetText - for a tweet, the content of the tweet.
+ * @property {string[]} tweetAttachments - for a tweet, any links attached to the tweet.
+ * @property {string} sharedId - for a favorite or share, the Twitter ID of the favorited or retweeted tweet.


Why not include a post ID for all Twitter, Facebook, and Reddit events?

jonathanmayer · 2021-06-21T07:26:32Z

src/socialMediaActivity.js

@@ -28,17 +29,155 @@ permissions.check({
 */


The permissions.check call above should go in the add listener callback functions, with the right match pattern and permissions (webRequestBlocking?) for the event type and listener options.

akohlbre and others added 3 commits April 8, 2021 04:02

create blocking events in socialMediaActivity

85c6874

documentation updates for socialMediaActivity

2169199

Merge branch 'main' into socialmedia-events

6df4e48