This is preparation for enabling an eslint indentation configuration.
90% of these changes are just fixes for indentation errors that have
snuck into the codebase over the years; the others are more
significant reformatting to make eslint happy (that are not otherwise
actually improvements).
The one area that we do not attempt to work on here is the
"switch/case" indentation.
If we have bugs in our compose code, we don't want them to
cause other strange side effects during reload. (We recently
had a test deployment that was a bit buggy, and the reload
problem was tough to chase down due to the misleading
symptoms.)
We sometimes get blueslip errors from browsers that are clearly still
attempting to reload long after they should have. These browsers can
produce a lot of unnecessary presence update exceptions.
To solve that, we start checking reload_in_progress in the presence
code path.
While we're at it, we also add some blueslip logging for the reload
code path, in case it becomes useful when debugging future issues.
We've had a few reports of users using modern Chrome having problems
where reload.is_in_progress() was true, but the browser was just
sitting there, not having reloaded.
This will continually attempt to reload the page periodically try and
compensate for the behavior in Chrome where it appears that the tab
has to be active or semi-active for `location.reload` to be respected
when Chrome is trying to save power, which means that it should just
continually try until the page is active again, in which case the
`location.reload` func will work and reload the page.
See https://developers.google.com/web/updates/2017/03/background_tabs
for the Chrome featureset that we believe may be involved with this
issue.
Tweaked by tabbott to reload earlier and add the on-focus handler.
Fixes: #6821.
This fixes an issue where browsers without local storage (aka the
Zulip ancient QT-based desktop app) would throw an exception trying to
reload in modern Zulip.
Previously, we'd log an exception whenever an invalid hashchange
reload token appeared, which is probably a bit excessive given that
this can happen without anything being wrong.
We instead just log something for debugging in the blueslip log and
make sure the #reload hash is cleared.
Despite the length of this commit, it is a very straightforward
moving of code from narrow.js -> narrow_state.js, and then
everything else is just s/narrow.foo()/narrow_state.foo()/
(with a few tiny cleanups to remove some code duplication
in certain callers).
The only new functions are simple setter/getters that
encapsulate the current_filter variable:
narrow_state.reset_current_filter()
narrow_state.set_current_filter()
narrow_state.get_current_filter()
We removed narrow.predicate() as part of this, since it was dead
code.
Also, we removed the shim for narrow_state.set_compose_defaults(),
and since that was the last shim, we removed shim.js from the app.
We now only call compose_state.composing() in a boolean context,
where we simply care whether or not the compose box is open. The
function now also returns true/false.
Callers who need to know the actual message type (e.g. "stream" or
"private") now call compose_state.get_message_type().
This is mostly just moving methods out of compose.js.
The variable `is_composing_message`, which isn't a boolean, has
been renamed to `message_type`, and there are new functions
set_message_type() and get_message_type() that wrap it.
This commit removes some shims related to the global variable
`compose_state`; now, `compose_state` is a typical global
variable with a 1:1 relationship with the module by the same
name.
The new module has 100% line coverage, most of it coming
via the tests on compose_actions.js. (The methods here are
super simple, so it's a good thing that the tests are somewhat
integrated with a higher layer.)
Zulip's logic for garbage-collecting data structures before reloading
was a fix for a Chrome memory leak that lasted across reloads a few
years ago. That memory leak is probably now fixed, and thus logic is
causing lots of JavaScript tracebacks that are probably not useful.
So, let's try removing this cleanup logic (everything but the
still-useful deletion of the event queue).
Apparently, Django's CSRF protection mechanism changed at some point,
and now we get a different CSRF token every time the webapp is loaded.
This, in turn, caused our reload logic to avoid losing state to be
completely ineffective, since the CSRF check in reload.initialize
always failed.
We fix this in a secure fashion by passing the reload instructions
from the browser to its reloaded self via localstorage, keyed by a
randomly generated token. The token randomization is primarily
relevant for handling several Zulip tabs in the same browser, but also
servers to make it very difficult for an attacker to ever trigger this
code path by redirecting a browser to `/#reload` URLs.
Fixes#3411.
Fixes#3687.
It's probably a bug that this throws exceptions sometimes, but it's
not consistently reproducible (or maybe, browser dependent?), and it
isn't really a problem to fail to abort some requests as part of the
page reload process; the abort was just there to make sure as little
as possible is happening so we can garbage-collect effectively.
Apparently it isn't always the case that removal of jquery and the DOM
prevents cleanup_event_queue from being called via the postunload
hook, so add a check to avoid it being double-called.
We are seeing error on CUSTOMER4 when clearing the DOM on reload. So
now we will only clear the message list.
(imported from commit f5d8d7d36cd1018f7def73ff9eda414387fcec5c)
The reload initiation is required to run before other parts of the site
are started so that page_params will be setup correctly. This moves that
initiation out of an on ready handler to an explicit initialize call
near where the rest of the app is started.
(imported from commit b8994311299327aa3cfa57e3d9e92124a47123f4)
When navigating away get_events will fail after we delete the event
queue before leaving the page. In that case we try to reload the page to
correct the problem. This happens before the page navigation is
complete, and then we reload zulip keeping the user captive. This was
only observed on on Firefox.
(imported from commit e001172e87a9f2ab7cf07a477e46b9d87752ac04)
Previously, we saved the current_msg_list selected id and then
restored it as the home_msg_list selected id, which could result in
the home view loading to the wrong place.
This takes some already bad code and makes it even more in need of
refactoring, but it does fix a pressing bug. We should definitely
refactor both:
* the top of narrow.js
* the save/restore code in reload.js
after this, though.
(imported from commit bb2040219e4f545ba90bb04a696996cec2831484)
The channel module now keeps track pending ajax requests and has an
abort_all function to angle all pending requests.
(imported from commit 4e78ab24d2295bd67de5633e3a200dfa489825b1)
Chrome was showing a memory leak after many auto-reloads. Emptying the
the collections and removing the event listeners reduces the severity.
Before this change 40 reloads would would grow to about 140MB, now it
stays around 50MB.
(imported from commit 55fbeff9bdd0363bb95929f2981a2de238ff35d8)
These engagement data will be useful both for making pretty graphs of
how addicted our users are as well as for allowing us to check whether
a new deployment is actually using the product or not.
This measures "number of minutes during which each user had checked
the app within the previous 15 minutes". It should correctly not
count server-initiated reloads.
It's possible that we should use something less aggressive than
mousemove; I'm a little torn on that because you really can check the
app for new messages without doing anything active.
This is somewhat tested but there are a few outstanding issues:
* Mobile apps don't report these data. It should be as easy as having
them send in update_active_status queries with new_user_input=true.
* The semantics of this should be better documented (e.g. the
management script should print out the spec above)x.
(imported from commit ec8b2dc96b180e1951df00490707ae916887178e)
In a few cases the $.each was doing something imperatively that was
terser and easier to understand by using a different Underscore method,
so a few of these I rewrote.
Some code was using the fact that jQuery sets `this` in the callback to
be the item; I rewrote those to use an explicit parameter.
Some code was using $(some selector).each(callback). I converted these
to _.each($(some selector), callback).
One function, ui.process_condensing, was written to be a jQuery $.each
callback despite being in a totally different module from code using it.
I noticed this and updated the function's args.
(imported from commit bf5922a35f257c168cc09ec1d077415d6ef19a03)
Shubham Dhama
Tim Abbott
Steve Howell
Brock Whittaker
Rishi Gupta
fionabunny
Umair Khan
Steve Howell
Rafid Aslam
lonerz
AZtheAsian
Vishnu Ks
Tim Abbott
Dylan Dumesnil
Jason Michalski
Tim Abbott
Scott Feeney
Tim Abbott