Previously, as soon as we decided to render linesToMeasure, we would
clear them out. However, if a second update interleaved with the update
that initially requested measurement, it could cause the requested lines
to not be present when the measurement phase from the first update
occurred. Now, any additional updates will only add to the set of lines
that need to be measured until the measurement phase actually happens.
Signed-off-by: Antonio Scandurra <as-cii@github.com>
This is a continuation of #15266. In that pull-request we managed to
prevent IME previews from being displayed in the editor when the
originating `keydown` event was default-prevented. However, it was still
possible for IME input to make it through the previous workarounds, thus
triggering the `textInput` event and showing unwanted text.
Pressing another key that would complete the in-progress IME input
would, in fact, first replace `this.lastKeydown` and then trigger the
`textInput` event. In the handling of that event we would detect
`this.lastKeydown` as "non-default-prevented" and therefore mistakenly
insert the IME text.
With this commit we are adopting a different strategy to mitigate the
issue. When receiving the wrong `compositionupdate` event we will first
disable the hidden input and then re-enable it on the next tick.
Disabling the input causes the in-progress IME input to be aborted and
the browser to never fire `textInput` nor `compositionupdate` events
anymore after that.
The only downside of this approach is that the hidden input also loses
focus, but we transfer it back to it as soon as the next tick of the
event loop is served and the input has been re-enabled.
When changing the editor styles, we force the component to remeasure
character dimensions. If they change, each line's height could change
too, causing the current scroll top position to not match the viewport
the user was observing. Thus, when detecting a line height change, we
try to show users the area of the screen they were looking prior to
tweaking the font size.
In trying to maintain the aforementioned logical position, however, we
were mistakenly scheduling a new update before actually finishing the
current one. This was problematic because if the first update detected
that the longest screen line changed and such line was off-screen, it
would try to render it. Before having the chance to measure it, though,
the new update would kick in and delete the new longest screen line
node, because it assumed it had already been measured. Finally, when
`measureContentDuringUpdateSync` fired, it would notice that the longest
screen line node did not exist and throw an exception as a result.
This commit changes the `updateSync` method to set the `updateScheduled`
flag only before returning control to the caller, as opposed to doing so
at the beginning. This prevents calls to `scheduleUpdate` made in
`updateSync` from scheduling new unwanted updates.
This avoids mysterious timing issues in which the editor gets a 'focus'
event in a state where `isVisible` returns false. If we always render
the hidden input, we can always focus it.
Signed-off-by: Antonio Scandurra <as-cii@github.com>
Previously, clicking on a block decoration to interact with it would
cause the editor to scroll to the line next to it. This is inconvenient,
especially if the decoration was designed to be interactive and
contained buttons or links. If the decoration was close to the bottom of
the screen, clicking on a button inside of it would make the editor
scroll down and abort the click.
This behavior regressed during the editor rendering layer rewrite and
with this commit we are restoring the original behavior by simply
ignoring clicks that land on block decorations.
Instead, if the measured line height equals 0, default it to 1 so that
the editor component doesn't start computing `NaN` or `Infinity` values
due to e.g. dividing by 0.
We should probably consider sanitizing line heights smaller than a
certain threshold, but that's non trivial because line height is
expressed as a multiplier of the font size. Also, users may style the
`line-height` property via CSS, which may still throw errors when using
small values.
Calling `pixelPositionForScreenPosition` was sometimes throwing an error
indicating that the requested position was not rendered and that, as
such, could not be measured.
This was caused by trying to measure a line that was visible at the
moment of the call while also having a pending autoscroll request that
would cause that line to go off-screen. Due to how the code was
structured, we would mistakenly detect that line as visible, autoscroll
to a different location, re-render a different region of the buffer and
then try to measure the now invisible line.
This commit fixes this issue by restructuring and simplifying the logic
for rendering extra lines in order to measure them. Now, every line for
which a measurement has been requested is stored in a `linesToMeasure`
map. During the first phase of the update process (after honoring
autoscroll requests), we detect which of these lines are currently
visible and if they're not, store them into the
`extraRenderedScreenLines` map, which is then used to render lines that
are invisible but need to be measured.
We use the value of the hidden input to display a preview of the
composition, but it might already contain spaces from previous
keystrokes, since we don't call preventDefault when spaces are inserted.
Etch's reconciliation routine causes elements to be sometimes
re-ordered. In order to move an element, however, Etch needs to first
detach it from the DOM and then re-append it at the right location.
This behavior is unacceptable for highlight decorations because it could
re-start CSS animations on a certain highlight decoration when a
completely different one is added or removed.
Even though we are still interested in restructuring etch's
reconciliation logic to prevent unwanted re-orderings, with this commit
we are switching to a custom routine to create/update/remove highlight
decorations that prevents unnecessary moves and, as a result, fixes the
undesired behavior described above.
On Linux, when the user performs a middle-button mouse click, Chromium
fires both a mouse-down event *and* a paste event. This commit teaches
the TextEditorComponent to ignore the paste event.
When the user performs a middle-mouse click on a tab, we get close the
tab and attempt to prevent Chromium's default processing for the event.
[1] This prevents Chromium's default processing for the *mouse down*
event, but then Chromium also fires a *paste* event, and that event
pastes the clipboard's current content into the newly-focused text
editor. 🙀
Since Atom already has its own logic for handling pasting, we
shouldn't (🤞) need to handle browser paste events. By ignoring the
browser paste events on Linux, we fixatom/tabs#461.
[1]
ce1d92e0ab/lib/tab-bar-view.coffee (L416-L418)
Previously, when `setModel` was called, we forgot to update the pointer
to the component in the newly supplied editor. This was causing the
element to not update in response to model updates but only as a result
of focus or visibility changes. We suspect this regressed during the
rewrite of the editor rendering layer.
With this commit we will now correctly swap the element's underlying
editor by updating the component pointer on the newly supplied editor.
Also, if the element was already attached to another editor, we will
null out the component reference on it, because one instance of
`TextEditorElement` can only represent one instance of `TextEditor`.
Previously, we were mistakenly not clearing out some styling properties
like `marginTop`, thus causing e.g. line numbers to be misaligned. This
was caused by manual updates to an element's style object, without a
consequent update to the NodePool.
With this commit we will now rely on `element.styleMap` (a DOM
primitive) to detect which styles have been set on a element that is
about to be recycled and, if the object being created doesn't use them,
simply clear them out.
Before rendering block decorations, we read their heights by putting
them into a special div called `blockDecorationMeasurementsArea`.
Previously, this div was not explicitly sized, which was causing
decorations to wrap while being measured but not when actually rendering
them.
This commit fixes this inconsistency by explicitly styling the
measurement area so that it has the same width as the component scroll
width.
By the time that the animation frame is delivered, the requested
autoscroll
position could not exist anymore. This could cause the editor component
to measure a non-existent line and, as a result, throw an exception.
With this commit we will always ignore measurements for screen lines
that do not exist.
