Open Bug 1296500 Opened 8 years ago Updated 2 years ago

Implement a new struct which has nsCOMPtr<nsINode> and offset in it

Categories

(Core :: DOM: Core & HTML, defect, P3)

defect

Tracking

()

Tracking Status
firefox51 --- affected

People

(Reporter: masayuki, Unassigned)

Details

Currently, a lot of methods return a node and an offset in the node. This causes the argument not simple. For example,

nsINode* GetNodePosition(nsINode* aNode, int32_t* aOffset);

If there is a mozilla global class like mozilla::NodePosition, such method can be:

NodePosition GetNodePosition(nsINode* aNode);

And if caller needs to check if it returns valid point, it should be able to call IsValid().

Currently, mozilla::EditorDOMPoint and mozilla::ContentEventHandler::NodePosition are similar to what I want. (Although, ContentEventHandler::NodePosition has additional members, so, new struct shouldn't be final.)

https://dxr.mozilla.org/mozilla-central/rev/97a52326b06a07930216ebefa5af333271578904/editor/libeditor/EditorUtils.h#261
https://dxr.mozilla.org/mozilla-central/rev/97a52326b06a07930216ebefa5af333271578904/dom/events/ContentEventHandler.h#107

Unfortunately, DOMPoint exists to represent a point in 2D field (x, y). So, "point" isn't clear for other developers. I think "NodePosition", "NodeAndOffset" or "NodeAndIndex" is a good name. However, for naming variables, I like "NodePosition".

Then, it should be defined and implemented in dom/base/NodePosition.h which should be exposed as "mozilla/NodePosition.h".

Any ideas?
Priority: -- → P3
What's wrong with EditorDOMPoint, other than the name?  I don't think a significant amount of code outside editor will want such a concept -- do you have any examples that justify putting it in dom/?  editor code is weird in wanting to deal with such things, I think.

NodePosition sounds to me like the position of a node, which this isn't, because it could be inside a text node.  I like NodeAndOffset just for explicitness, like FragmentOrElement, even though it's awkward.  Or maybe DOMPosition, but that might sound like a position that's used by DOM code instead of a position in the DOM.
I'm not talking about only in editor. In a lot of places, methods return both node and offset in it with reference or pointer arguments. I'd like to suggest a new choice at implementing such methods.

I like NodeAndOffset in your suggestion.
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.