Closed Bug 181105 Opened 21 years ago Closed 17 years ago

Selection behaves badly when SpellCheck Selection is on

Categories

(Core :: DOM: Selection, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: rods, Assigned: peterlubczynski-bugs)

References

Details

(Keywords: topembed+, Whiteboard: EDITORBASE+)

Attachments

(1 file, 1 obsolete file)

If I have a couple of words on a line highlighted with the SpellCheck selection
the "regualr" selection will only select the entire line.

If I try to double click on a word, or do a drag select, or even position the
caret and then hold the shift key down and use the arrow keys.
Blocks: 119232
Keywords: nsbeta1+, topembed+
Whiteboard: EDITORBASE
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.3alpha
Whiteboard: EDITORBASE → EDITORBASE+
QA Contact: pmac → sairuh
gotta fix this one.
Target Milestone: mozilla1.3alpha → mozilla1.5alpha
Blocks: PhtN5
unsetting target milestone
Target Milestone: mozilla1.5alpha → ---
I think I have a fix for this particular situation too but I see a few other
bugs afterwords:

In nsTextFrame.cpp, |DrawSelectionIterator::CurrentBackGroundColor| and
|DrawSelectionIterator::CurrentForeGroundColor| there are a few 'if' statements
that should do an 'and' rather than an 'or':

mTypes[mCurrentIdx] | nsISelectionController::SELECTION_NORMAL

...should be ...

mTypes[mCurrentIdx] & nsISelectionController::SELECTION_NORMAL
-->peterlubczynski
Assignee: mjudge → peterlubczynski
Status: ASSIGNED → NEW
This patch also fixes bug 220157.
Attachment #141155 - Flags: review?(bzbarsky)
Comment on attachment 141155 [details] [diff] [review]
Make alternate selection types render properly

>Index: content/base/src/nsSelection.cpp
>@@ -2792,17 +2792,37 @@ nsSelection::LookUpSelection(nsIContent 
>+      mDomSelections[j]->GetIsCollapsed(&iscollapsed);
>+      if (iscollapsed){

!iscollapsed, no?

r=bzbarsky with that.  Gotta love that code in nsFrame.

sfraser should take a look at this, probably....
Attachment #141155 - Flags: superreview?(sfraser)
Attachment #141155 - Flags: review?(bzbarsky)
Attachment #141155 - Flags: review+
Attached patch Better patchSplinter Review
Fixes the iscollapsed as mentioned above. I had to have the 'slow check' done
whenever one of the extra selections is set. Changing it to be better would
require more digging.
Attachment #141155 - Attachment is obsolete: true
Attachment #141155 - Flags: superreview?(sfraser)
Attachment #141176 - Flags: superreview?(sfraser)
Attachment #141176 - Flags: review?(bzbarsky)
It'll take me a few days to get to this (maybe till this coming weekend,
worst-case).
Comment on attachment 141176 [details] [diff] [review]
Better patch

>Index: content/base/src/nsSelection.cpp
>   PRInt8 j;
>+  for (j = (PRInt8) 1; j < (PRInt8)nsISelectionController::NUM_SELECTIONTYPES; j++){

Isn't the selection at "1" the normal selection?  So this patch always follows
the slow path when a selection exists?

>+      if (!iscollapsed){
>+        aSlowCheck = PR_TRUE;

and break here?

>Index: layout/html/base/src/nsFrame.cpp
>+          if (curDetail->mType != nsISelectionController::SELECTION_SPELLCHECK &&

This bizarreness should be documented in the selection controller IDL,
probably.
Attachment #141176 - Flags: review?(bzbarsky) → review-
> Isn't the selection at "1" the normal selection?  So this patch always follows
> the slow path when a selection exists?

Yes, but that's misleading since the normal path is the slow method and the slow
 path is the even slower method. Both methods will iterate over all ranges in
the selection every time a text frame is drawn which is incredibly slow with
lots of ranges. I haven't been able to come up with a good way of improving it.
Bug 220157 works for me now. That bug was probably fixed by the fix for bug 278312.
Maybe this bug is now also worksforme?
Yes, the inline spell code should have fixed this.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Attachment #141176 - Flags: superreview?(sfraser_bugs)
You need to log in before you can comment on or make changes to this bug.