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

NEW
Unassigned

Status

()

Core
Spelling checker
P1
normal
11 years ago
a year ago

People

(Reporter: Ria Klaassen (not reading all bugmail), Unassigned)

Tracking

(Depends on: 1 bug)

Trunk
Future
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [at risk])

Attachments

(3 attachments, 4 obsolete attachments)

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

Updated

11 years ago
Hardware: Other → All
(Reporter)

Updated

11 years ago
Hardware: All → PC
(Reporter)

Comment 1

11 years ago
This works fine in Thunderbird.
Assignee: mscott → nobody
Component: Spelling checker → General
Product: Core → Firefox
QA Contact: spelling-checker → general
Target Milestone: --- → Firefox 2
Version: 1.8 Branch → 2.0 Branch

Comment 2

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

Comment 3

11 years ago
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

11 years ago
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.
Flags: blocking-firefox2?

Updated

11 years ago
Assignee: nobody → brettw
Component: General → Spelling checker
Flags: blocking-firefox2?
OS: Windows XP → All
Priority: -- → P1
Product: Firefox → Core
Hardware: PC → All
Summary: Need to uncheck and check "Spell check this field" before the spell checker starts searching → Pasted or dragged text is not automatically re-spell checked
Target Milestone: Firefox 2 → mozilla1.8.1beta1
Version: 2.0 Branch → 1.8 Branch

Comment 5

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

Updated

11 years ago
Blocks: 338999

Updated

11 years ago
Blocks: 339066

Comment 6

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

Comment 7

11 years ago
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

11 years ago
(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

11 years ago
Created attachment 223364 [details]
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

11 years ago
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

11 years ago
Created attachment 223383 [details] [diff] [review]
Patch that doesn't work at all so I don't forget with I did
QA Contact: general → spelling-checker
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.

Updated

11 years ago
Flags: blocking1.9a1?
Whiteboard: swag: 12d

Updated

11 years ago
Flags: blocking1.8.1?

Comment 13

11 years ago
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.

Updated

11 years ago
Whiteboard: swag: 12d → swag: 4d

Comment 14

11 years ago
Created attachment 226391 [details] [diff] [review]
Word finder .cpp file
Attachment #223383 - Attachment is obsolete: true
Attachment #226391 - Flags: review?(roc)

Comment 15

11 years ago
Created attachment 226392 [details] [diff] [review]
Word finder .h file
Attachment #226392 - Flags: review?(roc)

Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+

Updated

11 years ago
Attachment #226391 - Flags: review?(roc)

Updated

11 years ago
Attachment #226392 - Flags: review?(roc)

Updated

11 years ago
Attachment #226391 - Attachment is obsolete: true

Updated

11 years ago
Attachment #226392 - Attachment is obsolete: true

Updated

11 years ago
Target Milestone: mozilla1.8.1beta1 → mozilla1.8.1beta2
No longer blocks: 339066
Depends on: 339066

Updated

11 years ago
Depends on: 345103

Comment 16

11 years ago
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.
Flags: blocking1.8.1+ → blocking1.8.1?
Whiteboard: swag: 4d → [at risk]
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.
Minusing for blocking1.8.1 per branch driving meeting because (1) it appears to be at least mostly working and (2) comment 16.
Flags: blocking1.8.1? → blocking1.8.1-

Updated

11 years ago
Flags: blocking1.9a1?

Comment 19

11 years ago
This is not going to get fixed any better for Fx2 at this point.
Assignee: brettw → mscott
No longer blocks: 338999
Target Milestone: mozilla1.8.1beta2 → Future
Version: 1.8 Branch → Trunk
This is fixed.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WORKSFORME

Comment 21

10 years ago
It works in most cases, but still breaks, depending on where and what you paste. Please see comment 16.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---

Updated

9 years ago
Assignee: mscott → nobody
Status: REOPENED → NEW

Comment 22

8 years ago
Still a bug. Bug 497511 has a nice testcase.
Depends on: 497511
Blocks: 758267

Updated

2 years ago
Duplicate of this bug: 367873

Comment 24

2 years ago
See nice test cases in attachment 8702046 [details].

Updated

2 years ago
Duplicate of this bug: 758267

Updated

2 years ago
Duplicate of this bug: 497511

Comment 27

2 years ago
Created attachment 8702047 [details]
Test page showing the bugs (from bug 918972).

Updated

2 years ago
No longer depends on: 497511

Updated

2 years ago
No longer blocks: 758267

Comment 28

2 years ago
Created attachment 8702070 [details]
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.
Attachment #8702047 - Attachment is obsolete: true
Flags: needinfo?(ehsan)

Updated

2 years ago
Duplicate of this bug: 918972

Comment 30

2 years ago
Created attachment 8702071 [details]
Test page showing the bug(s) (revised)
Attachment #8702070 - Attachment is obsolete: true

Comment 31

2 years ago
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.
Attachment #8702047 - Attachment is obsolete: false

Comment 32

2 years ago
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

2 years ago
I put the undo into the summary. Maybe we split it off into another spellcheck/undo bug. Also see bug 679011.
Summary: Pasted or dragged text is not automatically re-spell checked → Pasted or dragged text is not automatically re-spell checked. Also failing after undo.
(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.
Flags: needinfo?(ehsan)
You need to log in before you can comment on or make changes to this bug.