1394959 - Find API - don't wrap rectData with rectsAndText

Status: RESOLVED WONTFIX

(WebExtensions :: General, enhancement, P5)

Description

[Tim Nguyen :ntim]

Right now:

rectData looks like this:

rectData: {
  rectsAndTexts: {
    rect: [...],
    text: [...],
  }
}

I'm not sure wrapping rect, text with rectsAndTexts is useful here, given that there's nothing else in the object.

In case we do want to add more things in the object, then I do think the name "rectsAndTexts" could be improved

Comment 1

[u462496]

(In reply to Tim Nguyen :ntim from comment #0)
> Right now:
> 
> rectData looks like this:
> 
> rectData: {
>   rectsAndTexts: {
>     rect: [...],
>     text: [...],
>   }
> }
> 
> I'm not sure wrapping rect, text with rectsAndTexts is useful here, given
> that there's nothing else in the object.
> 
> In case we do want to add more things in the object, then I do think the
> name "rectsAndTexts" could be improved

rectsAndTexts is unique in that these arrays are aligned and have a special relationship with each other.  I feel it is something that shouldn't be messed with and should be kept as it is.

Comment 2

[Tim Nguyen :ntim]

(In reply to Kevin Jones from comment #1)
> rectsAndTexts is unique in that these arrays are aligned and have a special
> relationship with each other.  I feel it is something that shouldn't be
> messed with and should be kept as it is.

If we want to link the 2 together, we can:
- return a Map object
- return only one Array with: [{ rect: ..., text: ...}, etc.]

Comment 3

[u462496]

(In reply to Tim Nguyen :ntim from comment #2)
> (In reply to Kevin Jones from comment #1)
> > rectsAndTexts is unique in that these arrays are aligned and have a special
> > relationship with each other.  I feel it is something that shouldn't be
> > messed with and should be kept as it is.
> 
> If we want to link the 2 together, we can:
> - return a Map object
> - return only one Array with: [{ rect: ..., text: ...}, etc.]

rectsAndTexts is conveniently passed directly from FinderHighlighter.jsm._getRangeRectsAndTexts():

http://searchfox.org/mozilla-central/source/toolkit/components/extensions/FindContent.jsm#182-183

rectsAndTexts is used in the same construct in FinderHighlighter.jsm.  Why add the extra code?

I am not understanding why there is an issue being made with any of this.

Comment 4

[Tim Nguyen :ntim]

(In reply to Kevin Jones from comment #3)
> I am not understanding why there is an issue being made with any of this.

The API should be easy to understand no matter what the internal implementation is.

The link between the two should be easy to understand.

Comment 5

[Shane Caraveo (:mixedpuppy)]

I'm not convinced we need to do anything here.