The orig_initial_pointer variable was part of the implementation for
ensuring server-initiated reloads preserve the user's selected message
and scroll position (so that they are not disruptive). Previously,
the logic did some unnecessary contortions to ensure the two goals:
* The `pointer.js` logic knows what the server thinks the pointer is.
* The `message_fetch.js` logic knows what anchor to use to center it's
home view fetch.
It's a lot cleaner to do this by not mutating page_params.pointer.
This is not always a behavior-preserving translation: _.defaults
mutates its first argument. However, the code does not always appear
to have been written to expect that.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
We now treat util like a leaf module and
use "require" to import it everywhere it's used.
An earlier version of this commit moved
util into our "shared" library, but we
decided to wait on that. Once we're ready
to do that, we should only need to do a
simple search/replace on various
require/zrequire statements plus a small
tweak to one of the custom linter checks.
It turns out we don't really need util.js
for our most immediate code-sharing goal,
which is to reuse our markdown code on
mobile. There's a little bit of cleanup
still remaining to break the dependency,
but it's minor.
The util module still calls the global
blueslip module in one place, but that
code is about to be removed in the next
few commits.
I am pretty confident that once we start
sharing things like the typeahead code
more aggressively, we'll start having
dependencies on util. The module is barely
more than 300 lines long, so we'll probably
just move the whole thing into shared
rather than break it apart. Also, we
can continue to nibble away at the
cruftier parts of the module.
The compose_state.recipient field was only actually the recipient for
the message if it was a private_message_recipient (in the sense of
other code); we store the stream in compose_state.stream instead.
As a result, the name was quite confusing, resulting in the
possibility of problematic correctness bugs where code assumes this
field has a valid value for stream messages. Fix this by changing it
to compose_state.private_message_recipient for clarity.
This commit was originally automatically generated using `tools/lint
--only=eslint --fix`. It was then modified by tabbott to contain only
changes to a set of files that are unlikely to result in significant
merge conflicts with any open pull request, excluding about 20 files.
His plan is to merge the remaining changes with more precise care,
potentially involving merging parts of conflicting pull requests
before running the `eslint --fix` operation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Because of the separate declarations, ESLint would convert them to
`let` and then trigger the `prefer-const` error.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
With webpack, variables declared in each file are already file-local
(Global variables need to be explicitly exported), so these IIFEs are
no longer needed.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This is part of work to break some of our
nastier circular dependencies in preparation
for our es6 migration.
This commit should facilitate loading leaf-like
modules such as people.js before all of the things
that reload.js depends on.
This commit prepares the frontend code to be consumed by webpack.
It is a hack: In theory, modules should be declaring and importing the
modules they depend on and the globals they expose directly.
However, that requires significant per-module work, which we don't
really want to block moving our toolchain to webpack on.
So we expose the modules by setting window.varName = varName; as
needed in the js files.
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.
Anders Kaseorg
Tim Abbott
Tim Abbott
Anders Kaseorg
Steve Howell
Rohitt Vashishtha
Armaan Ahluwalia
Shubham Dhama
Brock Whittaker
Rishi Gupta
fionabunny
Umair Khan
Steve Howell
Rafid Aslam
lonerz
AZtheAsian
Vishnu Ks
Tim Abbott
Dylan Dumesnil