336719 - Pasted or dragged text is not automatically re-spell checked. Also failing after undo.

Status: NEW

(Core :: Spelling checker, defect, P3)

Description

[Ria Klaassen (not reading all bugmail)]

Steps to reproduce:
1. Paste this text into an input field:
Het dagblad baseerde zich op een verklaring van een rechtbank in de Servische hoofdstad Belgrado. Volgens Servische media zeggen de Servische autoriteiten inmiddels te weten waar Mladic zich tot eind 2005 in het land schuil hield. Toen raakte Belgrado het spoor kwijt, aldus de Servische minister van Economische Zaken, Bubalo.
2. Click in the field. I assume that a lot of errors (non-existing words in the en-us language) should be marked, but nothing happens.
3. Rightclick, uncheck "Spell check this field".
4. Rightclick, check "Spell check this field" again.
5. Finally most of the text will be underlined with dotted red.

Expectation: only steps 1 and 2 should be necessary (I assume).
NB: You can also make it work by clicking on each word separately.

Comment 1

[Ria Klaassen (not reading all bugmail)]

This works fine in Thunderbird.

Comment 2

[Adam Kowalczyk]

*** Bug 336819 has been marked as a duplicate of this bug. ***

Comment 3

[Adam Kowalczyk]

In other words: spellchecking is not run in all cases when text
is changed and needs checking. The cases that need to be covered are when the
text is pasted into the area and when it is dragged.

Comment 4

[Ben Basson]

This is confusing and counter-intuitive behaviour. I think it'd be nice to have (at least) a decision on whether this should be fixed for Firefox 2.0 or not. I personally think it should be, hence my comment.

The solutions that I can think of are:

1) Leave things as they are. This solves the performance issue of checking lots of pasted text at once. The behaviour is, however, confusing, since words inserted afterwards get checked and the pasted content does not.

2) Check pasted text only, as part of the pasting action, then carry on as normal (checking text as it is typed).

3) Check only the text visible within the text box (don't check text that isn't currently visible).

4) Add a "check now" item to the context menu.

I'm sure 2 and 3 aren't simple, but currently the only solution is to toggle on/off the inline spelling feature in order to check blocks of pasted text.

Is the perf issue a major concern here? I can understand the problem for large quantities of text, but do people paste large quantities of text into their browser on regular occasions?

4 isn't desirable at all, but it's better than toggling the checking feature on and off to achieve the objective.

Comment 5

[Brett Wilson]

*** Bug 332691 has been marked as a duplicate of this bug. ***

Comment 6

[Brett Wilson]

*** Bug 311514 has been marked as a duplicate of this bug. ***

Comment 7

[Doug Hockin]

My bug 311514 just got marked as a duplicate of this and I agree that's probably true.  But I'd like the people that work on it to have a look at the images I provided with my bug, as they show some things not made clear here.  I was pasting text into a mail message that I got from outside.  From the users point of view the spelling check did run on this new text, because it found "misspellings" in it and flagged them with a red underline.  What I reported was that some of these
red underlinings were only partial words.  Instead of underlining the whole word, it underlined only part of it.  Of course that partial word was misspelled, but the whole word was not.  So somehow it appears confused about where word boundaries are.  I had this happen again today, but unfortunately didn't
get a screen shot of it.  This time the word it made the mistake on (partial
red undeline) was
in the middle of a line in the middle of the pasted content.  I'll try to reproduce it and include an attachement here.

Comment 8

[Brett Wilson]

(In reply to comment #7)

Yes, the pasted content actually is in a new text node, resulting in a word spanning two text nodes. The current code doesn't handle this case. My patch to this bug will. Thanks.

Comment 9

[Doug Hockin]

Attachment: Image shows paste spellchecker errors

I created this by pasting the single line of text into the message body over and over (Ctrl-v).  There are two different spell checker errors displayed, both partial words, but most of the lines (correctly) show no error.

Comment 10

[Doug Hockin]

Check the image I just attached.  Those errors are in the middle of a pasted line
and unpredictable.  Each line inserted by a ctrl-v.

Comment 11

[Brett Wilson]

Attachment: Patch that doesn't work at all so I don't forget with I did

Comment 12

[Ria Klaassen (not reading all bugmail)]

When I go to this page http://en.wikipedia.org/w/index.php?title=Jimi_Hendrix&action=edit it is not always spell-checked. Sometimes it works, sometimes not. Just randomly.

Comment 13

[Brett Wilson]

daflk dlksd sf(In reply to comment #12)
> When I go to this page
> http://en.wikipedia.org/w/index.php?title=Jimi_Hendrix&action=edit it is not
> always spell-checked. Sometimes it works, sometimes not. Just randomly.

-> Filed bug 340328.

Comment 14

[Brett Wilson]

Attachment: Word finder .cpp file

Comment 15

[Brett Wilson]

Attachment: Word finder .h file

Comment 16

[Brett Wilson]

I do not see this getting fixed for 2.0. Can you please re-evaluate blocking?

The problem is that the editor does not tell me what changed. It might be possible to change the editor, but I am not really well-qualified to do that, and it's risky.

Sometimes it works when you paste, and sometimes it doesn't. The problem is that I only know where the cursor used to be, and where it is now. Sometimes this is the stuff that was pasted, and sometimes it is not.

When you paste at the end of a line, it will work, because I know the cursor used to be at the end of the line, and now it's after a bunch of text, so I know to check that.

If you paste in the middle of the line, it has to split that line's text node to insert the text. Often, the old text node ends up being the second one, and a new text node is created for the first part of the original node. The editor moves the cursor to the beginning of the second text node, which is the same position it was before as far as I can tell:

[        Node A       ]
        ^caret
[ Node B ][ Pasted text... ][ Node A ]
                             ^"previous" caret position

So when you paste, we get the previous and the current caret position to be at the end of what you pasted, so we don't know what to check.

Comment 17

[Ria Klaassen (not reading all bugmail)]

This is WFM 100% in trunk since a few weeks. At least when you paste normal portions of text. 
Don't know if there are still people seeing this in branch builds.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Minusing for blocking1.8.1 per branch driving meeting because (1) it appears to be at least mostly working and (2) comment 16.

Comment 19

[Brett Wilson]

This is not going to get fixed any better for Fx2 at this point.

Comment 20

[Ria Klaassen (not reading all bugmail)]

This is fixed.

Comment 21

[Brett Wilson]

It works in most cases, but still breaks, depending on where and what you paste. Please see comment 16.

Comment 22

[Matt Caywood]

Still a bug. Bug 497511 has a nice testcase.

Comment 24

[Jorg K (CEST = GMT+2)]

See nice test cases in attachment 8702046 [details].

Comment 27

[Jorg K (CEST = GMT+2)]

Attachment: Test page showing the bugs (from bug 918972).

Comment 28

[Jorg K (CEST = GMT+2)]

Attachment: Test page showing the bugs

Added another test case.

Ehsan, I've been browsing/triaging some spellcheck bugs. This is how I arrived here. This bug has got a good number of duplicates, so it might be about time it got fixed. Do you have any words of wisdom? Is there really a problem of notifications since this depends on bug 345103.

Comment 30

[Jorg K (CEST = GMT+2)]

Attachment: Test page showing the bug(s) (revised)

Comment 31

[Jorg K (CEST = GMT+2)]

Comment on attachment 8702047 [details]
Test page showing the bugs (from bug 918972).

Actually, the test copying "i donot read the thisis a Firefx Firefox" only fails as last test in the <div>, therefore I'll un-obsolete the attachment. It's all a bit weird and wonderful.

Comment 32

[Jorg K (CEST = GMT+2)]

Another observation: In attachment 8702071 [details] copy the "i donot read the thisis a Firefx Firefox" a few times. The spell marks show (unlike attachment 8702047 [details]). Try undo. That loses all spell check marks.

Comment 33

[Jorg K (CEST = GMT+2)]

I put the undo into the summary. Maybe we split it off into another spellcheck/undo bug. Also see bug 679011.

Comment 34

[(no longer active)]

(In reply to Jorg K (GMT+1) from comment #28)
> Ehsan, I've been browsing/triaging some spellcheck bugs. This is how I
> arrived here. This bug has got a good number of duplicates, so it might be
> about time it got fixed. Do you have any words of wisdom? Is there really a
> problem of notifications since this depends on bug 345103.

Sorry for the long delay here.  (I have given up the ownership of the editor code as you know, so I process these requests with a very low/unreliable frequency...)

I took a look at the code and it seems like this kind of thing is supposed to be handled by SpellCheckAfterEditorChange(), but we don't call that anywhere besides AfterEdit() which means it won't work for some things such as pasting.  So in a sense, this depends on bug 345103 in the sense that we need to somehow re-spellcheck after these things happen but I don't think we need to create an elaborate notification system.  Something to make spell checking work should be sufficient here.

Comment 35

[Sylvestre Ledru [:Sylvestre]]

Decreasing the priority as no update for the last 2 years on this bug.
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage 
about the priority meaning.

Comment 37

[Thomas D. (:thomas8)]

Hi Olli (:smaug, triage owner), I'm here on behalf of Thunderbird Team. As Jörg Knobloch has already pointed out 5 years ago (Comment 28) , this bug continues to get duplicates and it's one of those everyday pain points that makes Firefox and Thunderbird look bad, especially for the enterprise. Inline spellcheck becomes somewhat useless when it's not reliable and randomly deactivated after copy/paste, drag/drop, undo/redo operations.

• 15 years after this was filed, can we please agree on a plan of action to get this fixed soonish?
• Please raise the priority of this bug to P1 or P2, as the automated downgrade of priority from P1 to P5 (comment 35) was inappropriate.
• Kindly invite the right people/developers to look at this - Ehsan's comment 34 has an informed opinion where to start.

Comment 38

[Thomas D. (:thomas8)]

Here's a word-level variant of this, more likely to occur for languages like German were compound words are combined into one:

STR

1.) Type Die Polizei war nicht gut ausgestattet.
2.) Copy the word Station as plain text, e.g. from Notepad (to avoid other issues with HTML tags etc.)
3.) Place cursor directly after Polizei, press Ctrl+V to paste Station, navigate away using Cursor-right or END key (sic, do not use mouse click).
4.) Type some more, trying to trigger re-spellcheck, and now you can even click into the word or anywhere else.

Actual Result

Die PolizeiStation war nicht gut ausgestattet.

• PolizeiStation is now misspelled (compound single word, so the inner capital S is wrong), but doesn't get the red underline from spellcheck.
• There's almost nothing which will re-trigger the spellcheck on that word now.

Expected Result

• PolizeiStation should be marked as misspelled right after pasting, or at least after navigating out of the word with keyboard.

Comment 39

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

I see just one recent dup of this.

Sure, this is annoying, but nothing hints this should be P1. (though these days having both Priority and Severity fields make things a bit unclear).
I'd be very happy to review a patch, but may not have time to write it myself immediately.

Comment 40

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 8 duplicates.
:smaug, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.