These are the low-hanging-fruit places where we
can avoid using the helpers global.
The long term goal here is to make the markdown
code truly re-entrant, but some challenges still
remain.
Before this change, we would use **some** options relating
to parsing messages, but not all of them. The reason for
this was completely unintentional.
It's mostly a moot point, since the server sends back pretty
generic messages when you do something like invoke the
"/dark" command, and the message would parse the same way
whether or not the parser was looking for things like user
mentions or stream links.
In order to make this code predictable, I had to decide
whether we do a completely vanilla parse or a full message
parse. My decision now is mostly tactical. It's a trivial
one-line change to just use all the options for message
parsing, whereas it requires a major overhaul to allow a
vanilla parse.
I also predict that we will eventually want to parse these
server responses as if they were messages. I doubt the
zcommand responses would ever take advantage of it, but I
could imagine things like nag messages wanting to use user
mentions.
Even if my predictions are wrong, my decisions here are
pretty easy to reverse once we learn more.
For the particular case of zcommands, it is puzzling to me
why the server doesn't just send back HTML, but I don't want
to open that can of worms yet, as that would technically be
an API change.
For now I am happy with the one-line fix.
The zcommand code was calling directly into the "marked"
library, which was extremely misleading, since you don't
get a vanilla parse of the markdown due to the fact
that markdown.js calls setOptions at initialize time.
This commit shifts the responsibility to markdown.js
as well as adding a bit of test coverage, but it is
otherwise just a pure code-move refactoring.
The next commit will tweak things further.
This is mostly done for correctness reasons--it is
easiest from a logical standpoint to set the realm
emojis at the end of the function, since we do not want
them to be overwritten by normal emojis. The code
worked before this change, but it involved a clunky
check to map.has().
There is also probably a very minor performance
improvement insofar as N (the number of normal
emojis) is typically greater than R (the number
of realm emojis), and we eliminate N calls to
map.has in return for R calls to map.set. Even
if R is quite large, the readability advantages
probably far outweigh any performance considerations,
since we are using native map calls.
Thanks to Austin Riba for this suggestion.
The mobile app was never able to use the shared
version of emoji.js, because, among other problems
with our code organization, the emoji.js module
is strongly based on a mutate-the-data paradigm
that doesn't play nice with React. The way
that we mutate data and violate encapsuation
here is something that we would mostly want to fix
without even trying to shared code with mobile, so
subsequent commits will try to extract some pure
functions into a shared module.
The backend validates that URL inputs are RFC valid URLs (with no
specific length limit), but the frontend appears to have a maximum
length of 50 specified, likely because of a copy-paste error.
Increase the HTML maxlength for this input to 2048, which is a length
supported for URLs by all major browsers.
Fixes#21633
The feature deactivates the bot user; Zulip has no "delete bot"
feature. So fix the label to match what it does.
We also change the icon to match the one we use for deactivating users
in the "Manage users" UI.
Fixes#21619
We need to adjust height of recent topics along with the app
otherwise the container becomes separately scrollable due to
it overflowing the app height.
This is definitely better than having linkifiers
reach directly into marked.js, but there is
probably further improvement we can do here
to clean up how these regexes get set.
This introduces a circular dependency between
markdown.js and linkifiers.js, but we will
soon break it in the other direction.
Note we now avoid linkifier checks for the case that a message
contains more obvious backend-only syntax such as attachments.
The next commit will eliminate the ugly early-return.
This gets us closer to having an API that can
be used my mobile.
The parse() function becomes a subset of
apply_markdown() that is no longer coupled
to the shape of a webapp object, and it can
be supplied with a new helper_config for each
invocation. Mobile will likely call this directly.
The setup() function becomes a subset of
initialize() that allows you to set up the
parser **before** having to build any kind of
message-specific helpers. Mobile will likely
call this directly.
The webapp continues to call these functions,
which are now thin wrappers:
* apply_markdown (wrapping parse)
* initialize (wrapping setup)
Note we still have several other problems to
solve before mobile can use this code, but we
introduce this now so that we can get a head
start on prototyping and unit testing.
Also, this commit does not address the fact
that contains_backend_only_syntax() is still
bound to the webapp config.
We were showing the push notifications tooltip in user default
settings section even if the push notifications were configured
on the server.
The bug was because the setting value was undefined in the template
used for user default settings section, so this commit fixes the bug
by correctly passing the setting value to relevant template file.
Fixes#21602.
We use `fadeOut` effect to hide the scroll to bottom button more
widely. We already use `fadeIn` effect to display the button
every time.
We deliberately don't use the `fadeOut` effect when doing
`make_compose_box_full_size` to avoid any button overlap with
compose for a short time.
The implementation closely follows `handle_deactivation()`.
Using the same existing reactivate confirmation modal.
Also, this commit will also lead to open confirmation modal
to reactivate bots in settings > bots, and currently there is no
existing confirmation modal for deactivating bots.
This commit is a follow-up of #21436.
We previously added support for showing the status_emoji to the PM
list, but we only supported individual PMs and not group PMs, this is
prep commit towards supporting group PMs.
We previously added support for showing the status_emoji to the PM
list, but we only supported individual PMs and not group PMs, this is
prep commit towards supporting group PMs.
Previously, we were experiencing a regression in the positioning of
the play icon for youtube previews, as such, this commit uses a
previously created `handle_video_preview_mouseenter` to ensure the
positioning is always correct.
This is an ugly way of doing things, because this could be handled
directly through CSS flexbox, however, it is an acceptable temporary
fix.
This commit extracts the logic used to ensure that the play icon is
correctly positioned over the video preview for embedded videos, with
the intention that we can use this to fix a regression in the play
icon positioning for youtube video previews.
We render a login button for images that failed to load for
spectators. The image failed to load most likely due to being
rate limited by the server.
Fixes#19840
f0c680e9c0 introduced a call to
message_helper.process_new_message without first calling
message_store.set_message_flags on the message.
This resulted in it being possible as a race, when loading the Zulip
app to a stream/topic/near narrow, for a message to have the
`historical` flag be undefined due to not being initialized.
That invalid state, in turn, resulted in the message_list_view code
path for rendering the message feed incorrectly displaying additional
recipient bars around the message.
We could fix this by just calling message_store.set_message_booleans
in this code path. However, this bug exposes the fact that it's very
fragile to expect every code path to call that function before
message_helper.process_new_message.
So we instead fix this by moving message_store.set_message_booleans
inside message_helper.process_new_message.
One call point of concern in this change is maybe_add_narrow_messages,
which could theoretically reintroduce the double set_message_flags
bugs detailed in 9729b1a4ad. However, I
believe that to not be possible, because that call should never
experience a cache miss.
The other existing code paths were already calling
set_message_booleans immediately before
message_helper.process_new_message. They are still changing here, in
that we now do a cache lookup before attempting to call
set_message_booleans. Because the message booleans do not affect the
cache lookup and the local message object is discarded in case of a
cache hit, this should have no functional impact.
Because I found the existing comment at that call site confusing and
almost proposed removing it as pointless, extend the block comment to
explicitly mention that the purpose is refreshing our object.
Fixes#21503.
This eliminates an annoying bundle of complexity that caused the
frontend markdown processor's interface with the rest of Zulip's new
message processing code paths being more similar to that of a new
message from the server.
It also cuts down on code duplication.
The previous message.unread block in insert_local_message was
non-functional. markdown.apply_markdown is overriding what that set by
calling message_store.init_booleans, which happens to set the same
value for the `unread` flag, and then setting individual message
booleans as it finds elements like mentions during rendering.
Improve this situation partially by deleting the message_unread block,
and adding conversion logic to translate what the markdown processor
is doing into message flags.
Then, we can call message_store.set_message_flags just like we do when
processing new messages arriving via the API. This will be helpful
shortly, when we deduplicate the calls to
message_store.set_message_flags.
As noted in the lengthy TODO, this exposes the fact that we should
really rework how the frontend markdown processor returns the
mentioned state to its caller.
There is config_data for the embedded bots only for giphy and
followup bots, so we send "config_data" field to API only for
those bots and not others. Send config_data field as {} to
the API for other bots raised InternalError earlier.
On changing bot owner, "delete" event is sent to the previous
owner if previous owner is not an admin. We were ignoring the
"delete" event in webapp previously, but now we update the
bots page in personal settings to delete the bot. Note that
we do not remove the bot from the organization list of bots
currently, since list_widget does not support removing a
row as of now.
In case of previous owner being an admin, the previous owner
receives "update" event and thus the bots list is updated
from that event.
The code for ignoring "delete" event was added in fba2708bbc,
to basically avoid failed lookup for id in the organization
list of bots. I have tested and there cannot be a case of
a failed lookup in the list as per current code for list_widget
module and we are anyways safe after a reload or after closing the
overlay as the list will be updated correctly.
Discussion thread -
https://chat.zulip.org/#narrow/stream/321-settings-system/topic/List.20render.20.2315033Fixes#20856.
The "download" attribute on the button only functions for same-origin
requests; thus, the download endpoint must be used in order for the
"Download" button to function for uploaded images which are stored in
S3, and thus served from a different origin.
This is only done for uploaded images; it does not address a similar
problem with Camo, when Camo is hosted on a different hostname.
Fixes: #19238.
Currently, when deactivating a user, we have a "Saving..." loading
indicator and any error message displayed in the heading area of the
users table.
Migrate this to instead do the loading indicator and error message
inside the modal, where it's more in context.
As detailed in this conversation:
https://chat.zulip.org/#narrow/stream/137-feedback/topic/recent.20topics.20timestamps/near/1337670
This time format change is not working out as an improvement for at
least some users, myself included.
I think we do want to use some of the refinements attempted here (and
in particular, I'm keeping the new function with its nice test suite),
but I think it's better to revert now and fix forward in a future
release.
See #19775 for added background.
Render tooltip to stream_sorter_toggle buttons using
appendTo method of tippy.js to ensure that tooltip doesn't
get hidden behind the parent container and is visible
completely.
Fixes#21329.
This commit adds a tippy tooltip to the lightbox title which enables the
user to view the filename of an image if the filename is different than
the image title.
Fixes: #21333
This commit adds a tippy tooltip for inline image previews in messages.
There exists some (reasonable) logic in `static/js/util.js` which
overrides all title attributes for links to user-uploaded content to
ensure they always display "Download <filename>". This doesn't make
sense for inline images specifically because they will be opened in a
ligthbox, so we prevent that.
There is an additional tippy instance created in `static/js/tippyjs.js`
to add tippy tooltips to inline images, which takes advantage of the now
preserved title attribute of the parent link.
Creates a helper function in `message_edit.js` that loops over
a message's edit history to see if a stream and topic pair
existed at some point in the message history.
Exports `util.lower_same` function to use for comparing
edit history topics as lowercase.
Also adds test for new function in `mesage_edit` node tests.
We check if stream and topic present in the URL match that of
the message in its current state. If message is not available locally,
we fetch it before rendering the narrow.
Fixes#15290.
It turns out that the bug this call hopes to fix only happens when the
user first loads the page to recent_topics and then navigates to a
view with a message list (any other view), but we'd make this call
every time the recent topics table was hidden.
Hence, this commit makes it such that we only make that call if (1)
the page is loaded to recent_topics and (2) we're switching from
recent_topics to a message list view for the first time. We achieve
(1) via binding a handler via ui_init.initialize_everything and (2) by
binding the handler as `.one`, so that it's unbound after its first
invocation.
Additionally, we use window.requestAnimationFrame to prevent this
forcing the browser to do a reflow unnecessarily.
Combined with other commits in this series, this fixes a major
performance problem when leaving recent topics for another view.
See #20255 for details.
The name for a BigBlueButton meeting is now generated from the stream
name and topic name.
The createTime option is used to have the user redirected to a link
that is only valid for this meeting.
Even if the same link in Zulip is used again, a new createTime
parameter will be created, as the Meeting on the BigBlueButton server
has to be recreated.
Fixes#16498.
Fixes#20509.
Fixes#20804.
The previous logic didn't make sense -- the scroll offset in recent
topics is not a scroll offset within a message list, so saving it was
useless at best.
However, it was actually much worse than that, because trying to save
the pre-narrow offset while in the process of navigating away from
recent topics had the side effect of forcing a reflow, which resulted
in very expensive browser rendering to no purpose.
Adding this commit to the rest of the series of commits fixing
rendering issues when leaving recent topics, this commit results in an
impressive 3.05 s decrease in the first renarrow, an ~ 300 ms decrease
in the second renarrow and an ~ 500 ms decrease in the third renarrow.
There are still further forced reflows which could be reduced in this
render path, but they seem to not be as severe.
This is partially a prep commit to correctly saving/restoring the
position of the blue message select box when using browser
back/forward navigation, and partially a bug fix that ensures that
switching from "all_messages" to "recent_topics" preserves one's
position in "all_messages".
Note that this is with regards to saving the visual position of the
selected message, not about saving "which message was selected".
