Summary: | Move DOM Selection operations out of SelectionController | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||
Component: | HTML Editing | Assignee: | Alexey Proskuryakov <ap> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | ||||||
Priority: | P3 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Alexey Proskuryakov
2007-11-12 07:18:11 PST
Created attachment 17202 [details]
proposed patch
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
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). |