From 5aff2a6957e861162d67c4610277e9bb2a6f8186 Mon Sep 17 00:00:00 2001 From: ThibG Date: Fri, 29 May 2020 16:14:16 +0200 Subject: Fix timeline markers not working on Chrome (#13887) * Periodically save timeline markers This saves timeline markers immediately upon message arrival, but not more than once every 5 minutes. This does not change how the markers are saved on closing the window, except that it avoids submitting them if there is no need for it. * Use the Fetch API when possible instead of XHR on window unload --- app/javascript/mastodon/actions/markers.js | 73 ++++++++++++++++++++---- app/javascript/mastodon/actions/notifications.js | 4 ++ app/javascript/mastodon/actions/timelines.js | 9 +++ app/javascript/mastodon/features/ui/index.js | 4 +- app/javascript/mastodon/reducers/index.js | 2 + app/javascript/mastodon/reducers/markers.js | 25 ++++++++ 6 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 app/javascript/mastodon/reducers/markers.js (limited to 'app/javascript') diff --git a/app/javascript/mastodon/actions/markers.js b/app/javascript/mastodon/actions/markers.js index c3a5fe86f..9cc061a87 100644 --- a/app/javascript/mastodon/actions/markers.js +++ b/app/javascript/mastodon/actions/markers.js @@ -1,30 +1,81 @@ -export const submitMarkers = () => (dispatch, getState) => { +import api from '../api'; +import { debounce } from 'lodash'; +import compareId from '../compare_id'; +import { showAlertForError } from './alerts'; + +export const MARKERS_SUBMIT_SUCCESS = 'MARKERS_SUBMIT_SUCCESS'; + +export const synchronouslySubmitMarkers = () => (dispatch, getState) => { const accessToken = getState().getIn(['meta', 'access_token'], ''); - const params = {}; + const params = _buildParams(getState()); + + if (Object.keys(params).length === 0) { + return; + } + + if (window.fetch) { + fetch('/api/v1/markers', { + keepalive: true, + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${accessToken}`, + }, + body: JSON.stringify(params), + }); + } else { + const client = new XMLHttpRequest(); + + client.open('POST', '/api/v1/markers', false); + client.setRequestHeader('Content-Type', 'application/json'); + client.setRequestHeader('Authorization', `Bearer ${accessToken}`); + client.SUBMIT(JSON.stringify(params)); + } +}; - const lastHomeId = getState().getIn(['timelines', 'home', 'items', 0]); - const lastNotificationId = getState().getIn(['notifications', 'items', 0, 'id']); +const _buildParams = (state) => { + const params = {}; - if (lastHomeId) { + const lastHomeId = state.getIn(['timelines', 'home', 'items', 0]); + const lastNotificationId = state.getIn(['notifications', 'items', 0, 'id']); + + if (lastHomeId && compareId(lastHomeId, state.getIn(['markers', 'home'])) > 0) { params.home = { last_read_id: lastHomeId, }; } - if (lastNotificationId) { + if (lastNotificationId && compareId(lastNotificationId, state.getIn(['markers', 'notifications'])) > 0) { params.notifications = { last_read_id: lastNotificationId, }; } + return params; +}; + +const debouncedSubmitMarkers = debounce((dispatch, getState) => { + const params = _buildParams(getState()); + if (Object.keys(params).length === 0) { return; } - const client = new XMLHttpRequest(); + api().post('/api/v1/markers', params).then(() => { + dispatch(submitMarkersSuccess(params)); + }).catch(error => { + dispatch(showAlertForError(error)); + }); +}, 300000, { leading: true, trailing: true }); + +export function submitMarkersSuccess({ home, notifications }) { + return { + type: MARKERS_SUBMIT_SUCCESS, + home: (home || {}).last_read_id, + notifications: (notifications || {}).last_read_id, + }; +}; - client.open('POST', '/api/v1/markers', false); - client.setRequestHeader('Content-Type', 'application/json'); - client.setRequestHeader('Authorization', `Bearer ${accessToken}`); - client.send(JSON.stringify(params)); +export function submitMarkers() { + return (dispatch, getState) => debouncedSubmitMarkers(dispatch, getState); }; diff --git a/app/javascript/mastodon/actions/notifications.js b/app/javascript/mastodon/actions/notifications.js index 8a066b896..a26844f84 100644 --- a/app/javascript/mastodon/actions/notifications.js +++ b/app/javascript/mastodon/actions/notifications.js @@ -7,6 +7,7 @@ import { importFetchedStatus, importFetchedStatuses, } from './importer'; +import { submitMarkers } from './markers'; import { saveSettings } from './settings'; import { defineMessages } from 'react-intl'; import { List as ImmutableList } from 'immutable'; @@ -70,6 +71,8 @@ export function updateNotifications(notification, intlMessages, intlLocale) { filtered = regex && regex.test(searchIndex); } + dispatch(submitMarkers()); + if (showInColumn) { dispatch(importFetchedAccount(notification.account)); @@ -157,6 +160,7 @@ export function expandNotifications({ maxId } = {}, done = noOp) { dispatch(expandNotificationsSuccess(response.data, next ? next.uri : null, isLoadingMore, isLoadingRecent, isLoadingRecent && preferPendingItems)); fetchRelatedRelationships(dispatch, response.data); + dispatch(submitMarkers()); }).catch(error => { dispatch(expandNotificationsFail(error, isLoadingMore)); }).finally(() => { diff --git a/app/javascript/mastodon/actions/timelines.js b/app/javascript/mastodon/actions/timelines.js index 00a7a6789..de1725acf 100644 --- a/app/javascript/mastodon/actions/timelines.js +++ b/app/javascript/mastodon/actions/timelines.js @@ -1,4 +1,5 @@ import { importFetchedStatus, importFetchedStatuses } from './importer'; +import { submitMarkers } from './markers'; import api, { getLinks } from 'mastodon/api'; import { Map as ImmutableMap, List as ImmutableList } from 'immutable'; import compareId from 'mastodon/compare_id'; @@ -36,6 +37,10 @@ export function updateTimeline(timeline, status, accept) { status, usePendingItems: preferPendingItems, }); + + if (timeline === 'home') { + dispatch(submitMarkers()); + } }; }; @@ -98,6 +103,10 @@ export function expandTimeline(timelineId, path, params = {}, done = noOp) { const next = getLinks(response).refs.find(link => link.rel === 'next'); dispatch(importFetchedStatuses(response.data)); dispatch(expandTimelineSuccess(timelineId, response.data, next ? next.uri : null, response.status === 206, isLoadingRecent, isLoadingMore, isLoadingRecent && preferPendingItems)); + + if (timelineId === 'home') { + dispatch(submitMarkers()); + } }).catch(error => { dispatch(expandTimelineFail(timelineId, error, isLoadingMore)); }).finally(() => { diff --git a/app/javascript/mastodon/features/ui/index.js b/app/javascript/mastodon/features/ui/index.js index 957e80737..81ffad22e 100644 --- a/app/javascript/mastodon/features/ui/index.js +++ b/app/javascript/mastodon/features/ui/index.js @@ -16,7 +16,7 @@ import { expandNotifications } from '../../actions/notifications'; import { fetchFilters } from '../../actions/filters'; import { clearHeight } from '../../actions/height_cache'; import { focusApp, unfocusApp } from 'mastodon/actions/app'; -import { submitMarkers } from 'mastodon/actions/markers'; +import { synchronouslySubmitMarkers } from 'mastodon/actions/markers'; import { WrappedSwitch, WrappedRoute } from './util/react_router_helpers'; import UploadArea from './components/upload_area'; import ColumnsAreaContainer from './containers/columns_area_container'; @@ -251,7 +251,7 @@ class UI extends React.PureComponent { handleBeforeUnload = e => { const { intl, dispatch, isComposing, hasComposingText, hasMediaAttachments } = this.props; - dispatch(submitMarkers()); + dispatch(synchronouslySubmitMarkers()); if (isComposing && (hasComposingText || hasMediaAttachments)) { // Setting returnValue to any string causes confirmation dialog. diff --git a/app/javascript/mastodon/reducers/index.js b/app/javascript/mastodon/reducers/index.js index b9817cd38..3823bb05e 100644 --- a/app/javascript/mastodon/reducers/index.js +++ b/app/javascript/mastodon/reducers/index.js @@ -35,6 +35,7 @@ import identity_proofs from './identity_proofs'; import trends from './trends'; import missed_updates from './missed_updates'; import announcements from './announcements'; +import markers from './markers'; const reducers = { announcements, @@ -73,6 +74,7 @@ const reducers = { polls, trends, missed_updates, + markers, }; export default combineReducers(reducers); diff --git a/app/javascript/mastodon/reducers/markers.js b/app/javascript/mastodon/reducers/markers.js new file mode 100644 index 000000000..2e67be82e --- /dev/null +++ b/app/javascript/mastodon/reducers/markers.js @@ -0,0 +1,25 @@ +import { + MARKERS_SUBMIT_SUCCESS, +} from '../actions/notifications'; + +const initialState = ImmutableMap({ + home: '0', + notifications: '0', +}); + +import { Map as ImmutableMap } from 'immutable'; + +export default function markers(state = initialState, action) { + switch(action.type) { + case MARKERS_SUBMIT_SUCCESS: + if (action.home) { + state = state.set('home', action.home); + } + if (action.notifications) { + state = state.set('notifications', action.notifications); + } + return state; + default: + return state; + } +}; -- cgit From cc650bc023e00d07c5796b7602d86597bb437f45 Mon Sep 17 00:00:00 2001 From: ThibG Date: Fri, 29 May 2020 19:25:57 +0200 Subject: Fix timeline markers in Firefox (regression from #13887) (#13889) Unfortunately, Firefox does not support the `keepalive` parameter I used in the previous PR. However it supports the `navigator.sendBeacon` API that allows that kind of things, but does not allow setting headers. Therefore, this PR replaces it with a `sendBeacon` call that passes the bearer token in the POST data. Doorkeeper will then handle the auth token out of the box, as long as it is passed as form data. Passing the query as JSON does not work. --- app/javascript/mastodon/actions/markers.js | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'app/javascript') diff --git a/app/javascript/mastodon/actions/markers.js b/app/javascript/mastodon/actions/markers.js index 9cc061a87..37d1ddccf 100644 --- a/app/javascript/mastodon/actions/markers.js +++ b/app/javascript/mastodon/actions/markers.js @@ -13,7 +13,10 @@ export const synchronouslySubmitMarkers = () => (dispatch, getState) => { return; } - if (window.fetch) { + // The Fetch API allows us to perform requests that will be carried out + // after the page closes. But that only works if the `keepalive` attribute + // is supported. + if (window.fetch && 'keepalive' in new Request('')) { fetch('/api/v1/markers', { keepalive: true, method: 'POST', @@ -23,13 +26,31 @@ export const synchronouslySubmitMarkers = () => (dispatch, getState) => { }, body: JSON.stringify(params), }); - } else { + return; + } else if (navigator && navigator.sendBeacon) { + // Failing that, we can use sendBeacon, but we have to encode the data as + // FormData for DoorKeeper to recognize the token. + const formData = new FormData(); + formData.append('bearer_token', accessToken); + for (const [id, value] of Object.entries(params)) { + formData.append(`${id}[last_read_id]`, value.last_read_id); + } + if (navigator.sendBeacon('/api/v1/markers', formData)) { + return; + } + } + + // If neither Fetch nor sendBeacon worked, try to perform a synchronous + // request. + try { const client = new XMLHttpRequest(); client.open('POST', '/api/v1/markers', false); client.setRequestHeader('Content-Type', 'application/json'); client.setRequestHeader('Authorization', `Bearer ${accessToken}`); client.SUBMIT(JSON.stringify(params)); + } catch (e) { + // Do not make the BeforeUnload handler error out } }; -- cgit From 9bd30b8dd563d1c7066060ccfa456ea350a7fb01 Mon Sep 17 00:00:00 2001 From: ThibG Date: Fri, 29 May 2020 16:14:16 +0200 Subject: [Glitch] Fix timeline markers not working on Chrome Port 5aff2a6957e861162d67c4610277e9bb2a6f8186 to glitch-soc Signed-off-by: Thibaut Girka --- app/javascript/flavours/glitch/actions/markers.js | 70 ++++++++++++++++++---- .../flavours/glitch/actions/notifications.js | 4 ++ .../flavours/glitch/actions/timelines.js | 9 +++ .../flavours/glitch/features/ui/index.js | 4 +- app/javascript/flavours/glitch/reducers/index.js | 2 + app/javascript/flavours/glitch/reducers/markers.js | 25 ++++++++ 6 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 app/javascript/flavours/glitch/reducers/markers.js (limited to 'app/javascript') diff --git a/app/javascript/flavours/glitch/actions/markers.js b/app/javascript/flavours/glitch/actions/markers.js index 7ffab404d..b237d82ec 100644 --- a/app/javascript/flavours/glitch/actions/markers.js +++ b/app/javascript/flavours/glitch/actions/markers.js @@ -1,38 +1,86 @@ import api from 'flavours/glitch/util/api'; +import { debounce } from 'lodash'; +import compareId from 'flavours/glitch/util/compare_id'; +import { showAlertForError } from './alerts'; export const MARKERS_FETCH_REQUEST = 'MARKERS_FETCH_REQUEST'; export const MARKERS_FETCH_SUCCESS = 'MARKERS_FETCH_SUCCESS'; export const MARKERS_FETCH_FAIL = 'MARKERS_FETCH_FAIL'; +export const MARKERS_SUBMIT_SUCCESS = 'MARKERS_SUBMIT_SUCCESS'; -export const submitMarkers = () => (dispatch, getState) => { +export const synchronouslySubmitMarkers = () => (dispatch, getState) => { const accessToken = getState().getIn(['meta', 'access_token'], ''); - const params = {}; + const params = _buildParams(getState()); - const lastHomeId = getState().getIn(['timelines', 'home', 'items', 0]); - const lastNotificationId = getState().getIn(['notifications', 'lastReadId']); + if (Object.keys(params).length === 0) { + return; + } + + if (window.fetch) { + fetch('/api/v1/markers', { + keepalive: true, + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${accessToken}`, + }, + body: JSON.stringify(params), + }); + } else { + const client = new XMLHttpRequest(); + + client.open('POST', '/api/v1/markers', false); + client.setRequestHeader('Content-Type', 'application/json'); + client.setRequestHeader('Authorization', `Bearer ${accessToken}`); + client.SUBMIT(JSON.stringify(params)); + } +}; + +const _buildParams = (state) => { + const params = {}; - if (lastHomeId) { + const lastHomeId = state.getIn(['timelines', 'home', 'items', 0]); + const lastNotificationId = state.getIn(['notifications', 'lastReadId']); + + if (lastHomeId && compareId(lastHomeId, state.getIn(['markers', 'home'])) > 0) { params.home = { last_read_id: lastHomeId, }; } - if (lastNotificationId && lastNotificationId !== '0') { + if (lastNotificationId && lastNotificationId !== '0' && compareId(lastNotificationId, state.getIn(['markers', 'notifications'])) > 0) { params.notifications = { last_read_id: lastNotificationId, }; } + return params; +}; + +const debouncedSubmitMarkers = debounce((dispatch, getState) => { + const params = _buildParams(getState()); + if (Object.keys(params).length === 0) { return; } - const client = new XMLHttpRequest(); + api().post('/api/v1/markers', params).then(() => { + dispatch(submitMarkersSuccess(params)); + }).catch(error => { + dispatch(showAlertForError(error)); + }); +}, 300000, { leading: true, trailing: true }); + +export function submitMarkersSuccess({ home, notifications }) { + return { + type: MARKERS_SUBMIT_SUCCESS, + home: (home || {}).last_read_id, + notifications: (notifications || {}).last_read_id, + }; +}; - client.open('POST', '/api/v1/markers', false); - client.setRequestHeader('Content-Type', 'application/json'); - client.setRequestHeader('Authorization', `Bearer ${accessToken}`); - client.send(JSON.stringify(params)); +export function submitMarkers() { + return (dispatch, getState) => debouncedSubmitMarkers(dispatch, getState); }; export const fetchMarkers = () => (dispatch, getState) => { diff --git a/app/javascript/flavours/glitch/actions/notifications.js b/app/javascript/flavours/glitch/actions/notifications.js index b3de7b5bf..ceb1e6df6 100644 --- a/app/javascript/flavours/glitch/actions/notifications.js +++ b/app/javascript/flavours/glitch/actions/notifications.js @@ -7,6 +7,7 @@ import { importFetchedStatus, importFetchedStatuses, } from './importer'; +import { submitMarkers } from './markers'; import { saveSettings } from './settings'; import { defineMessages } from 'react-intl'; import { List as ImmutableList } from 'immutable'; @@ -81,6 +82,8 @@ export function updateNotifications(notification, intlMessages, intlLocale) { filtered = regex && regex.test(searchIndex); } + dispatch(submitMarkers()); + if (showInColumn) { dispatch(importFetchedAccount(notification.account)); @@ -168,6 +171,7 @@ export function expandNotifications({ maxId } = {}, done = noOp) { dispatch(expandNotificationsSuccess(response.data, next ? next.uri : null, isLoadingMore, isLoadingRecent, isLoadingRecent && preferPendingItems)); fetchRelatedRelationships(dispatch, response.data); + dispatch(submitMarkers()); }).catch(error => { dispatch(expandNotificationsFail(error, isLoadingMore)); }).finally(() => { diff --git a/app/javascript/flavours/glitch/actions/timelines.js b/app/javascript/flavours/glitch/actions/timelines.js index 46b605e04..9a7f62a08 100644 --- a/app/javascript/flavours/glitch/actions/timelines.js +++ b/app/javascript/flavours/glitch/actions/timelines.js @@ -1,4 +1,5 @@ import { importFetchedStatus, importFetchedStatuses } from './importer'; +import { submitMarkers } from './markers'; import api, { getLinks } from 'flavours/glitch/util/api'; import { Map as ImmutableMap, List as ImmutableList } from 'immutable'; import compareId from 'flavours/glitch/util/compare_id'; @@ -49,6 +50,10 @@ export function updateTimeline(timeline, status, accept) { usePendingItems: preferPendingItems, filtered }); + + if (timeline === 'home') { + dispatch(submitMarkers()); + } }; }; @@ -112,6 +117,10 @@ export function expandTimeline(timelineId, path, params = {}, done = noOp) { dispatch(importFetchedStatuses(response.data)); dispatch(expandTimelineSuccess(timelineId, response.data, next ? next.uri : null, response.status === 206, isLoadingRecent, isLoadingMore, isLoadingRecent && preferPendingItems)); + + if (timelineId === 'home') { + dispatch(submitMarkers()); + } }).catch(error => { dispatch(expandTimelineFail(timelineId, error, isLoadingMore)); }).finally(() => { diff --git a/app/javascript/flavours/glitch/features/ui/index.js b/app/javascript/flavours/glitch/features/ui/index.js index 9f9ef561a..f8f6cff88 100644 --- a/app/javascript/flavours/glitch/features/ui/index.js +++ b/app/javascript/flavours/glitch/features/ui/index.js @@ -12,7 +12,7 @@ import { expandHomeTimeline } from 'flavours/glitch/actions/timelines'; import { expandNotifications, notificationsSetVisibility } from 'flavours/glitch/actions/notifications'; import { fetchFilters } from 'flavours/glitch/actions/filters'; import { clearHeight } from 'flavours/glitch/actions/height_cache'; -import { submitMarkers, fetchMarkers } from 'flavours/glitch/actions/markers'; +import { synchronouslySubmitMarkers, fetchMarkers } from 'flavours/glitch/actions/markers'; import { WrappedSwitch, WrappedRoute } from 'flavours/glitch/util/react_router_helpers'; import UploadArea from './components/upload_area'; import PermaLink from 'flavours/glitch/components/permalink'; @@ -267,7 +267,7 @@ class UI extends React.Component { handleBeforeUnload = (e) => { const { intl, dispatch, hasComposingText, hasMediaAttachments } = this.props; - dispatch(submitMarkers()); + dispatch(synchronouslySubmitMarkers()); if (hasComposingText || hasMediaAttachments) { // Setting returnValue to any string causes confirmation dialog. diff --git a/app/javascript/flavours/glitch/reducers/index.js b/app/javascript/flavours/glitch/reducers/index.js index 586b84749..852abe9dd 100644 --- a/app/javascript/flavours/glitch/reducers/index.js +++ b/app/javascript/flavours/glitch/reducers/index.js @@ -36,6 +36,7 @@ import polls from './polls'; import identity_proofs from './identity_proofs'; import trends from './trends'; import announcements from './announcements'; +import markers from './markers'; const reducers = { announcements, @@ -75,6 +76,7 @@ const reducers = { pinnedAccountsEditor, polls, trends, + markers, }; export default combineReducers(reducers); diff --git a/app/javascript/flavours/glitch/reducers/markers.js b/app/javascript/flavours/glitch/reducers/markers.js new file mode 100644 index 000000000..2e67be82e --- /dev/null +++ b/app/javascript/flavours/glitch/reducers/markers.js @@ -0,0 +1,25 @@ +import { + MARKERS_SUBMIT_SUCCESS, +} from '../actions/notifications'; + +const initialState = ImmutableMap({ + home: '0', + notifications: '0', +}); + +import { Map as ImmutableMap } from 'immutable'; + +export default function markers(state = initialState, action) { + switch(action.type) { + case MARKERS_SUBMIT_SUCCESS: + if (action.home) { + state = state.set('home', action.home); + } + if (action.notifications) { + state = state.set('notifications', action.notifications); + } + return state; + default: + return state; + } +}; -- cgit From 9707dbee6fbbf8a5fa3c92762588a5405364acc6 Mon Sep 17 00:00:00 2001 From: ThibG Date: Fri, 29 May 2020 19:25:57 +0200 Subject: [Glitch] Fix timeline markers in Firefox Port cc650bc023e00d07c5796b7602d86597bb437f45 to glitch-soc Signed-off-by: Thibaut Girka --- app/javascript/flavours/glitch/actions/markers.js | 25 +++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'app/javascript') diff --git a/app/javascript/flavours/glitch/actions/markers.js b/app/javascript/flavours/glitch/actions/markers.js index b237d82ec..96e29accf 100644 --- a/app/javascript/flavours/glitch/actions/markers.js +++ b/app/javascript/flavours/glitch/actions/markers.js @@ -16,7 +16,10 @@ export const synchronouslySubmitMarkers = () => (dispatch, getState) => { return; } - if (window.fetch) { + // The Fetch API allows us to perform requests that will be carried out + // after the page closes. But that only works if the `keepalive` attribute + // is supported. + if (window.fetch && 'keepalive' in new Request('')) { fetch('/api/v1/markers', { keepalive: true, method: 'POST', @@ -26,13 +29,31 @@ export const synchronouslySubmitMarkers = () => (dispatch, getState) => { }, body: JSON.stringify(params), }); - } else { + return; + } else if (navigator && navigator.sendBeacon) { + // Failing that, we can use sendBeacon, but we have to encode the data as + // FormData for DoorKeeper to recognize the token. + const formData = new FormData(); + formData.append('bearer_token', accessToken); + for (const [id, value] of Object.entries(params)) { + formData.append(`${id}[last_read_id]`, value.last_read_id); + } + if (navigator.sendBeacon('/api/v1/markers', formData)) { + return; + } + } + + // If neither Fetch nor sendBeacon worked, try to perform a synchronous + // request. + try { const client = new XMLHttpRequest(); client.open('POST', '/api/v1/markers', false); client.setRequestHeader('Content-Type', 'application/json'); client.setRequestHeader('Authorization', `Bearer ${accessToken}`); client.SUBMIT(JSON.stringify(params)); + } catch (e) { + // Do not make the BeforeUnload handler error out } }; -- cgit