Bug 15954 – Move DOM Selection operations out of SelectionController

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Alexey Proskuryakov]

From bug 15922 comment 2:

> DOMSelection is the DOM's "selection object" API. It has the same relationship
> to SelectionController that DOMWindow has to Frame. Right now there's a problem
> in that we were "grooming" SelectionController to be this DOM object. So it has
> many functions that should simply be moved into DOMSelection.

Patch forthcoming.

[Alexey Proskuryakov]

Created attachment 17202 [details]
proposed patch

[Darin Adler]

Comment on attachment 17202 [details]
proposed patch

+    if (selectionController->isNone())
+        return "None";
+    else if (selectionController->isCaret())
+        return "Caret";
+    else
+        return "Range";

We normally don't do else after return.

+        || node->offsetInCharacters() && offset > caretMaxOffset(node)
+        || !node->offsetInCharacters() && offset > (int)node->childNodeCount()) {

Could probably rewrite this using ? : to not call offsetInCharacters twice. Or maybe there should be a helper function for this relatively common operation?

+        // Safari Selection Object API

Wow, I'm really surprised to learn that baseNode is Safari-specific!

Great patch! r=me

[Alexey Proskuryakov]

Committed revision 27744.

(In reply to comment #2)
> We normally don't do else after return.

Fixed.

> Could probably rewrite this using ? : to not call offsetInCharacters twice.

Done.

> Or maybe there should be a helper function for this relatively common operation?

That's something I'd like to avoid - currently, there seem to be too many editing helper functions that aren't quite clear about what they do, because the concept of a position is messed up (editing positions vs. DOM ones vs. rendering ones).