Bug 1663583 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Alessandro Castellani (:aleca) from comment #4)
> Review of attachment 9176586 [details] [diff] [review] ⧉[details] [diff] [review]:
> ::: mail/components/compose/content/addressingWidgetOverlay.js
> @@ +549,5 @@
> > +    case " ":
> > +      // Prevent the typing of a blank space as a first character.
> > +      if (!input.value.trim()) {
> > +        event.preventDefault();
> > +      }
> 
> This doesn't work as the white space was already typed as we're not
> listening to a "before" keydown, but after the keydown.

Ummm, no sir. At the time of keydown event, the new character isn't visible yet in the input, and input.value has the *previous* (unchanged) value. `console.log (input.value);` can tell. So this works exactly as expected (and as we already have it in the other function).

> We should clear the field instead of preventDefault which doesn't do
> anything.

preventDefault() is needed to prevent the space from being added in this event.

*In addition*, yes, we can clear any pre-existing whitespace, which might temporarily occur in the rare event that
- whitespace-only like "   " was pasted and focus still in the input, or
- whitespace-only like "  " is a volatile leftover from having " foo ", deleting or cutting "foo" and then remaining with "  " for as long as focus remains in the input.

We only prevent entering a space and clear the previous value IF the *previous* input.value is whitespace-only, because we have to allow changing "foo" to " foo" or "foo ". I rewrote the comment to be more accurate.

> this is good, but we should add the macos metaKey modifier also here since
> we forgot

Yes.
(In reply to Alessandro Castellani (:aleca) from comment #4)
> Review of attachment 9176586 [details] [diff] [review] ⧉[details] [diff] [review]:
> ::: mail/components/compose/content/addressingWidgetOverlay.js
> @@ +549,5 @@
> > +    case " ":
> > +      // Prevent the typing of a blank space as a first character.
> > +      if (!input.value.trim()) {
> > +        event.preventDefault();
> > +      }
> 
> This doesn't work as the white space was already typed as we're not
> listening to a "before" keydown, but after the keydown.

Ummm, no sir. At the time of keydown event, the new character isn't visible yet in the input, and input.value has the *previous* (unchanged) value. `console.log (input.value);` can tell. So this works exactly as expected (and as we already have it in the other function).

> We should clear the field instead of preventDefault which doesn't do
> anything.

preventDefault() is needed to prevent the space from being added in this event.

*In addition*, yes, we can clear any pre-existing whitespace, which might temporarily occur in the rare event that
- whitespace-only like "   " was pasted and focus still in the input, or
- whitespace-only like "  " is a volatile leftover from having " foo ", deleting or cutting "foo" and then remaining with "  " for as long as focus remains in the input.

We only prevent entering a space and clear the previous value IF the *previous* input.value is whitespace-only, because we have to allow changing "foo" to " foo" or "foo ". I rewrote the comment to be more accurate.

> this is good, but we should add the macos metaKey modifier also here since
> we forgot

Yes.
(In reply to Alessandro Castellani (:aleca) from comment #4)

> Review of attachment 9176586 [details] [diff] [review] ⧉[details] [diff] [review]:
> ::: mail/components/compose/content/addressingWidgetOverlay.js
> @@ +549,5 @@
```
> > +    case " ":
> > +      // Prevent the typing of a blank space as a first character.
> > +      if (!input.value.trim()) {
> > +        event.preventDefault();
> > +      }
```
> 
> This doesn't work as the white space was already typed as we're not
> listening to a "before" keydown, but after the keydown.

Ummm, no sir. At the time of keydown event, the new character isn't visible yet in the input, and input.value has the *previous* (unchanged) value. `console.log (input.value);` can tell. So this works exactly as expected (and as we already have it in the other function).

> We should clear the field instead of preventDefault which doesn't do
> anything.

preventDefault() is needed to prevent the space from being added in this event.

*In addition*, yes, we can clear any pre-existing whitespace, which might temporarily occur in the rare event that
- whitespace-only like "   " was pasted and focus still in the input, or
- whitespace-only like "  " is a volatile leftover from having " foo ", deleting or cutting "foo" and then remaining with "  " for as long as focus remains in the input.

We only prevent entering a space and clear the previous value IF the *previous* input.value is whitespace-only, because we have to allow changing "foo" to " foo" or "foo ". I rewrote the comment to be more accurate.

> this is good, but we should add the macos metaKey modifier also here since
> we forgot

Yes.

Back to Bug 1663583 Comment 5