(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.
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.